Make flexbox automatically preffed off by default, in release builds

VERIFIED FIXED in Firefox 20

Status

()

Core
Layout
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

Trunk
mozilla21
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox18 unaffected, firefox19 unaffected, firefox20+ fixed, relnote-firefox 20+)

Details

Attachments

(2 attachments)

(Assignee)

Description

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

Updated

5 years ago
Summary: Disable flexbox support in release builds → Make flexbox automatically preffed off by default, in release builds
(Assignee)

Comment 1

5 years ago
Created attachment 714541 [details] [diff] [review]
patch
Attachment #714541 - Flags: review?(dbaron)
(Assignee)

Updated

5 years ago
Blocks: 841876
(Assignee)

Comment 2

5 years ago
(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.)
Attachment #714541 - Flags: review?(dbaron) → review+
(Assignee)

Comment 3

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

Comment 4

5 years ago
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.
status-firefox18: --- → unaffected
status-firefox19: --- → unaffected
tracking-firefox20: --- → ?
status-firefox20: --- → affected
tracking-firefox20: ? → +
relnote-firefox: --- → ?
relnote-firefox: ? → 20+
Marking relnote 20+ to remind myself that this will have to be removed from the FF20 release notes.
(Assignee)

Comment 6

5 years ago
(ah, thanks -- I was going to ask about that.)
(Assignee)

Comment 7

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

Comment 8

5 years ago
Created attachment 714809 [details] [diff] [review]
test-changes patch

(Attaching test-changes patch, for reference)
(Assignee)

Comment 9

5 years ago
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
Attachment #714541 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 10

5 years ago
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)
Attachment #714809 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/1fae6592d663
https://hg.mozilla.org/mozilla-central/rev/b682c1433cbf
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
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.
Attachment #714541 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 13

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/32a3591ea826
https://hg.mozilla.org/releases/mozilla-aurora/rev/9abc752f17f5
status-firefox20: affected → fixed
(Assignee)

Updated

5 years ago
Attachment #714809 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 14

5 years ago
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.