Closed Bug 797022 Opened 8 years ago Closed 8 years ago
_FLEXBOX in builds
Filing this bug on enabling the MOZ_FLEXBOX build flag so that the flexbox code will be built into nightlies. (but still preffed off, per bug 796212, so the default user-experience shouldn't change)
This bug is going to be in 3 parts. The main part (attached here) is enabling MOZ_FLEXBOX in builds. There are also two test parts: - tweaks to the flexbox mochitests, to have them turn on the pref manually. (I've got a patch for that mostly working). - enable & tweak the flexbox reftests, to have them turn on the pref manually via their manifest The ordering will be: part 1: tweak mochitests part 2: enable MOZ_FLEXBOX part 3: tweak reftest manifest (The ordering may seem odd, but there's a reason for it: - The mochitests will start running as soon as the MOZ_FLEXBOX build flag is enabled, so they need to be aware of the pref before we enable the build flag, so that they'll set the pref and run correctly. - The reftests, on the other hand, are currently disabled by having their manifest file commented out; as soon as it's commented back in, the reftests will start running. So we can't uncomment it until after MOZ_FLEXBOX is enabled.)
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #667384 - Flags: review?(bzbarsky)
Here's the reftests patch. 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. (c) Uncomments the line for the flexbox manifest in the master layout/reftests/reftest.list file. (NOTE: in the pref-enabling annotations, "test-pref()" means "only toggle the pref for the testcase, not for the reference case", whereas "pref()" toggles it for both. I use "test-pref()" in most cases because most of the reftests only use flexbox features in the testcase and not in the reference.)
...and here's part 1, to get the mochitests enabling the pref so that they'll do the right thing once the build flag is on w/ the pref default-disabled. The idea here is to push each flexbox-specific mochitest into an iframe, instead of running them directly, so we can enable the pref before we load them. (The dom bindings effects of the pref seem to attach at document-load-time -- so I have to enable the pref before loading the document that contains the actual test code.) I haven't sanity-checked this part 100% yet, so I'm not requesting review on this one yet.
Attachment #667384 - Attachment description: patch 2: enable MOZ_FLEXBOX in builds, and uncomment idl → part 2: enable MOZ_FLEXBOX in builds, and uncomment idl
Attachment #667387 - Attachment description: patch 3: enable reftests and make them toggle the pref as-needed → part 3: enable reftests and make them toggle the pref as-needed
Comment on attachment 667401 [details] [diff] [review] part 1: toggle/check the pref as-needed in mochitests Actually, I think it makes sense to take care of the unit tests in a different bug. Filed bug 797601 for that, and this bug will be a nice and simple build-flag-enabling.
Attachment #667401 - Attachment is obsolete: true
(I'm doing the reftest.list-uncommenting as part of this bug's patch, though, because we can't do that until the build flag is enabled)
Attachment #667702 - Flags: review?(bzbarsky)
Comment on attachment 667702 [details] [diff] [review] fix: enable MOZ_FLEXBOX in builds (and uncomment flexbox chunks in IDL and reftest.list) Rev the iid like the comment says, and r=me.
Attachment #667702 - Flags: review?(bzbarsky) → review+
oops, thanks for catching that! Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/7ad0763694a3
OS: Linux → All
Hardware: x86_64 → All
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.