Closed Bug 797601 Opened 7 years ago Closed 7 years ago

Tweak flexbox unit tests to support being run with flexbox pref default-disabled

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(3 files)

No description provided.
This patch moves the flexbox chunks of property_database.js and test_transitions_per_property.html from a commented-out chunk to a pref-controlled chunk.

This means that generally (with the pref disabled), mochitests won't see the flex properties listed in property_database.js, which is good.

And:
 (a) if we selectively enable the pref before loading a test (as my next patch does), then that test will see the flexbox properties in property_database.js, as it would like to.

 (b) when we enable the pref by default in all.js, these mochtiests will transparently Just Work.  (and then at some point after that, we can remove the pref-checking code)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
For each existing test_flexbox_$FOO.html, this patch renames the test to file_flexbox_$FOO.html and creates a new lightweight test_flexbox_$FOO.html that loads the "file_" one in an iframe. (after enabling the pref)
Attachment #667727 - Flags: review?(bzbarsky)
Attachment #667712 - Flags: review?(bzbarsky)
bugzilla's diff viewer gets a little confused by these since there's "hg cp" involved. The first half of patch is for the new "file_flexbox_$FOO.html" copies, and the second half is for the lightweight test_flexbox_$FOO.html harnesses that I'm swapping in for them.

bz: feel free to just rubber-stamp these based on comment 2 and a bit of skimming, if you like.
Here's the reftest-tweak patch.

Copypasting the description I'd originally posted in bug 797022 comment #2 (before I spun off this separate bug for the test tweaks):
> This patch:
>  (a) Adds a reftest, with a "display: none; display: -moz-flex" element, as
> a test for the pref behaving properly.  I've added reference cases for what
> it should look like when the pref is on & when it's off, and I test both
> cases.
> 
>  (b) Adds annotations to the flexbox reftest.list file to toggle the pref as
> needed.  (Just adding prefixes to all of the existing lines there)  This is
> a bit messy, but it only needs to be there for as long as the pref is
> disabled by default.
Attachment #667728 - Flags: review?(bzbarsky)
Comment on attachment 667712 [details] [diff] [review]
part 1: check pref in property_table.js and test_transitions_per_property.html

r=me
Attachment #667712 - Flags: review?(bzbarsky) → review+
Comment on attachment 667727 [details] [diff] [review]
part 2: selectively enable the pref in flexbox mochitests

r=me
Attachment #667727 - Flags: review?(bzbarsky) → review+
Comment on attachment 667728 [details] [diff] [review]
part 3: add a reftest for flexbox pref, and add pref-enabling annotations to reftest.list

r=me
Attachment #667728 - Flags: review?(bzbarsky) → review+
OS: Linux → All
Hardware: x86_64 → All
You need to log in before you can comment on or make changes to this bug.