Closed
Bug 1483685
Opened 6 years ago
Closed 6 years ago
Add "start"/"end" to flexbox-align-content-horizrev / vertrev test files, and move them into upstreamable w3c directory
Categories
(Core :: Layout, enhancement)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: dholbert, Assigned: dholbert)
References
Details
Attachments
(2 files)
12.00 KB,
patch
|
mihir
:
review+
|
Details | Diff | Splinter Review |
10.86 KB,
patch
|
mihir
:
review+
|
Details | Diff | Splinter Review |
Now that we've fixed bug 1472843, we can flesh out the reftest files...
flexbox-align-content-horizrev-001.xhtml
flexbox-align-content-vertrev-001.xhtml
...to test start/end values (which they currently don't test, in part because we used to get the rendering wrong, I think, but we'll get it right now that bug 1472843's fix has landed.
These tests are basically identical to the files flexbox-align-content-horiz-001a.xhtml (and its -vert- equivalent), and Mihir extended those tests in bug 1472843 to test 'start' and 'end', so we should make these same extensions to these other tests.
Also: Mihir's extensions sprinkled around some "wrap-reverse" for robustness, but we don't need to do that now that we'll have a dedicated test for wrap-reverse. So we can roll back that part of his test tweaks, I think.
I've got some tweaks for this locally, so I'll post them shortly and have Mihir review to sanity-check my changes, if he's got cycles before the end of his internship.
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #9001406 -
Flags: review?(miyer)
Assignee | ||
Comment 2•6 years ago
|
||
This patch just edits the test files to remove the "wrap-reverse" styling, and edits the reference cases to copypaste the "flex-start"/"flex-end" reference chunks to replace our "start/end" reference chunks.
(In this case, the "flex-start" and "start" side are the same, because there's no reversal going on anymore. But I still had to update the reference case because the multi-flexrow cases are now stacked in the opposite order, as a/b/c rather than c/b/a, now that I've removed the row-reversing style in the testcases.)
Attachment #9001415 -
Flags: review?(miyer)
Updated•6 years ago
|
Attachment #9001406 -
Flags: review?(miyer) → review+
Updated•6 years ago
|
Attachment #9001415 -
Flags: review?(miyer) → review+
Assignee | ||
Comment 3•6 years ago
|
||
Thanks for the review!
I made sure yesterday that tests pass locally in both affected reftest directories. Here's a try run with a few other platforms, just in case:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f96d256b089941cf118f4a9c20260a847d3653d9
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(dholbert)
Assignee | ||
Comment 4•6 years ago
|
||
Comment on attachment 9001406 [details] [diff] [review]
part 1: Move flexbox-align-content-horizrev-001.xhtml (and its vertrev variant) to w3c-css directory, and add "start/end" cases inside it.
Review of attachment 9001406 [details] [diff] [review]:
-----------------------------------------------------------------
I just realized I used the wrong bug number in the commit messages here:
> Bug 1472843 part 1: Move flexbox-align-content-horizrev-001.xhtml (and its vertrev variant) to w3c-css directory, and add "start/end" cases inside it. r?mihir
Oops -- copypaste error. :) I'll fix that before landing.
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7400a9b3fb35
part 1: Move flexbox-align-content-horizrev-001.xhtml (and its vertrev variant) to w3c-css directory, and add "start/end" cases inside it. r=mihir
https://hg.mozilla.org/integration/mozilla-inbound/rev/c22409215ce2
part 2: Remove wrap-reverse styling from flexbox-align-content-horiz-001* reftest (& 'vert' variant) since that's now tested elsewhere. r=mihir
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(dholbert)
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7400a9b3fb35
https://hg.mozilla.org/mozilla-central/rev/c22409215ce2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
web-platform-tests PR in https://github.com/web-platform-tests/wpt/pull/12552
You need to log in
before you can comment on or make changes to this bug.
Description
•