Last Comment Bug 841873 - Make flexbox automatically preffed off by default, in release builds
: Make flexbox automatically preffed off by default, in release builds
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla21
Assigned To: Daniel Holbert [:dholbert]
:
Mentors:
Depends on: 814530
Blocks: 841876
  Show dependency treegraph
 
Reported: 2013-02-15 12:39 PST by Daniel Holbert [:dholbert]
Modified: 2013-02-25 09:34 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
unaffected
+
fixed
20+


Attachments
patch (1.06 KB, patch)
2013-02-15 12:42 PST, Daniel Holbert [:dholbert]
dbaron: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
test-changes patch (9.48 KB, patch)
2013-02-16 10:41 PST, Daniel Holbert [:dholbert]
no flags Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2013-02-15 12:39:05 PST
Our CSS3 flexbox support isn't quite production-ready, so I think we should keep flexbox support preffed off *in release builds* for the time being (but preffed on in nightly & aurora).

We can use the "#ifdef RELEASE_BUILD" functionality from bug 814530 to achieve this.

(I'll file a followup bug on removing this #ifdef -- to enable it in release builds. That bug will track the bugs that need fixing before we ship it in official releases.)
Comment 1 Daniel Holbert [:dholbert] 2013-02-15 12:42:07 PST
Created attachment 714541 [details] [diff] [review]
patch
Comment 2 Daniel Holbert [:dholbert] 2013-02-15 13:14:21 PST
(In reply to Daniel Holbert [:dholbert] from comment #0)
> (I'll file a followup bug on removing this #ifdef -- to enable it in release
> builds. That bug will track the bugs that need fixing before we ship it in
> official releases.)

(I filed bug 841876 on this, BTW.)
Comment 3 Daniel Holbert [:dholbert] 2013-02-15 14:44:10 PST
This will need a patch to add pref() / test-pref() statements to test manifests, to revert the "tweak reftests/mochitests accordingly" chunk of bug 783409's patch.
Comment 4 Daniel Holbert [:dholbert] 2013-02-15 15:23:44 PST
Flagging as "tracking-firefox20?" to be sure this ends up making it there.

(Firefox 20 is currently on aurora, w/ the pref enabled (from bug 783409).  It moves to beta next week, and it needs this patch in order for the pref to be default-disabled there.)

Ideally it'd be great to have this landed on aurora before the switchover, but it'd also be fine to land it on beta afterwards, as long as it's not too delayed.

In the meantime, I'm going to whip up the test/test-manifest-tweaks that I alluded to in comment 3 & get this landed on trunk.
Comment 5 Lukas Blakk [:lsblakk] use ?needinfo 2013-02-15 15:37:50 PST
Marking relnote 20+ to remind myself that this will have to be removed from the FF20 release notes.
Comment 6 Daniel Holbert [:dholbert] 2013-02-15 15:44:21 PST
(ah, thanks -- I was going to ask about that.)
Comment 7 Daniel Holbert [:dholbert] 2013-02-16 09:55:57 PST
(In reply to Daniel Holbert [:dholbert] from comment #3)
> This will need a patch to add pref() / test-pref() statements to test
> manifests, to revert the "tweak reftests/mochitests accordingly" chunk of
> bug 783409's patch.

Landed a patch to do that (after landing I realized that the commit message unnecessarily mentioned crashtests, but no crashtests were actually modified in the production of this cset):
  https://hg.mozilla.org/integration/mozilla-inbound/rev/1fae6592d663

And, I landed the attached patch:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/b682c1433cbf
Comment 8 Daniel Holbert [:dholbert] 2013-02-16 10:41:33 PST
Created attachment 714809 [details] [diff] [review]
test-changes patch

(Attaching test-changes patch, for reference)
Comment 9 Daniel Holbert [:dholbert] 2013-02-16 10:44:55 PST
Comment on attachment 714541 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 783409 (turned pref on) 

User impact if declined: see comment 0

Testing completed (on m-c, etc.): verified locally that relevant reftests/crashtests/mochitests still pass, w/ both possible pref values.

Risk to taking this patch (and alternatives if risky): very low. (just keeping a pref in the same position as it was in previous release)

String or UUID changes made by this patch: none
Comment 10 Daniel Holbert [:dholbert] 2013-02-16 10:47:02 PST
Comment on attachment 714809 [details] [diff] [review]
test-changes patch

[requesting approval for the test-changes patch, too. This just removes assumptions in our tests about what the flexbox pref's default value is.]

(maybe test-only patches don't need approval? I know that used to be the case, but I forget if that changed)
Comment 12 Lukas Blakk [:lsblakk] use ?needinfo 2013-02-18 17:37:50 PST
Comment on attachment 714541 [details] [diff] [review]
patch

Approving for Aurora uplift - Merge day is tomorrow (tues) starting at 6am Pacific time and this should land before we build FF20 beta 1 so please try to land this to either FF20 on Aurora now or prepare to uplift to mozilla-beta branch post-merge, before 12pm PT tomorrow.
Comment 14 Daniel Holbert [:dholbert] 2013-02-25 09:34:36 PST
I just updated my beta build to version 20 (via Help | About), and I verified that this pref is disabled by default there.

--> Marking Verified Fixed.

Mozilla/5.0 (X11; Linux x86_64; rv:20.0) Gecko/20100101 Firefox/20.0

Note You need to log in before you can comment on or make changes to this bug.