Closed Bug 1483685 Opened 2 years ago Closed 2 years ago

Add "start"/"end" to flexbox-align-content-horizrev / vertrev test files, and move them into upstreamable w3c directory

Categories

(Core :: Layout, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(2 files)

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.
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)
Attachment #9001406 - Flags: review?(miyer) → review+
Attachment #9001415 - Flags: review?(miyer) → review+
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
Flags: needinfo?(dholbert)
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
Flags: needinfo?(dholbert)
https://hg.mozilla.org/mozilla-central/rev/7400a9b3fb35
https://hg.mozilla.org/mozilla-central/rev/c22409215ce2
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Marking this as qe-verify -, per comment 7.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.