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)

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)
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: