Enable MOZ_FLEXBOX in builds

RESOLVED FIXED in mozilla18

Status

()

defect
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Assignee

Description

7 years ago
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)
Assignee

Comment 1

7 years ago
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)
Assignee

Comment 2

7 years ago
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.)
Attachment #667387 - Flags: review?(bzbarsky)
Assignee

Comment 3

7 years ago
...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.
Assignee

Updated

7 years ago
Attachment #667384 - Attachment description: patch 2: enable MOZ_FLEXBOX in builds, and uncomment idl → part 2: enable MOZ_FLEXBOX in builds, and uncomment idl
Assignee

Updated

7 years ago
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
Assignee

Updated

7 years ago
Depends on: 797601
Assignee

Comment 4

7 years ago
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
Assignee

Updated

7 years ago
Attachment #667387 - Attachment is obsolete: true
Attachment #667387 - Flags: review?(bzbarsky)
Assignee

Comment 5

7 years ago
(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 #667384 - Attachment is obsolete: true
Attachment #667384 - Flags: review?(bzbarsky)
Assignee

Updated

7 years ago
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+
Assignee

Comment 7

7 years ago
oops, thanks for catching that!

Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/7ad0763694a3
OS: Linux → All
Hardware: x86_64 → All
https://hg.mozilla.org/mozilla-central/rev/7ad0763694a3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Assignee

Updated

6 years ago
Depends on: 864553
You need to log in before you can comment on or make changes to this bug.