Closed Bug 936100 Opened 6 years ago Closed 6 years ago

Remove about:config pref "layout.css.flexbox.enabled"

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [DocArea=CSS][good first verify])

Attachments

(6 files)

We've shipped four releases now with the flexbox pref turned on by default (22, 23, 24, 25).  I think it's safe to say we're not going to toggle it off at this point, so we should remove the pref.
(Note also that we have "display: flex" in UI code all over the place now...
 http://mxr.mozilla.org/mozilla-central/search?string=display%3A%20flex
...so this pref is a bit of a footgun that will presumably break parts of the browser UI if users toggle it off.)
Patch stack coming up.

(Not that it matters super-much, but for the record: at each intermediate point in this patch stack, we'll pass tests.)

First, I'll remove pref tweaking / usage in unit tests; then, I'll remove the C++ code that deals with the pref; finally, I'll remove the line from all.js.
This removes manifest annotations for the pref, along with some testcases whose sole purpose was to assert that toggling the pref on & off made a difference in rendering.
Attachment #829695 - Flags: review?(matspal)
This patch removes flexbox-pref-specific stuff from some "all-property"-ish mochitests.
Attachment #829696 - Flags: review?(matspal)
Attachment #829696 - Attachment description: part 2: fix up general mochitests → part 2: Adjust non-flexbox-specific mochitests to no longer bother with flexbox pref
So, the flexbox-specific mochitests are currently set up like so:
  file_XYZ.html --> contains the actual test logic
  test_XYZ.html --> shim that sets the pref, & loads file_XYZ.html in iframe

We had this setup because, before the pref was on by default, we needed to make sure to toggle the pref on *before* document-load-time in order for it to be respected.

In any case - now, we can drop the "test_" shims, and just rename the file_* shims to be the actual tests.

This patch does all of that, *except* for the renaming -- i.e. this deletes the existing test_* files and copies a bit of mochitest boilerplate from them over to the file_* files.  (The next patch will do the renaming.)
Attachment #829697 - Flags: review?(matspal)
Here's the next part, to rename the file_* files to test_*.
Attachment #829698 - Flags: review?(matspal)
And finally, this removes the pref from all.js.

(At this point in the patch stack, no more mentions of the pref exist in the codebase.)
Attachment #829700 - Flags: review?(matspal)
Attachment #829695 - Flags: review?(matspal) → review+
Attachment #829696 - Flags: review?(matspal) → review+
Attachment #829697 - Flags: review?(matspal) → review+
Attachment #829698 - Flags: review?(matspal) → review+
Attachment #829699 - Flags: review?(matspal) → review+
Comment on attachment 829700 [details] [diff] [review]
part 6: remove pref from all.js

r=mats  Thanks for the helpful comments.
Attachment #829700 - Flags: review?(matspal) → review+
teoli, RE dev-doc-needed -- what documentation changes are needed?

From a quick search, it looks like the only mentions of the pref on MDN are regarding old versions of Firefox where the pref was default-disabled, anyway. For example:
  To activate flexbox support for Firefox 18 and 19, the user has to
  change the about:config preference "layout.css.flexbox.enabled" to true.
(At the bottom of https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Flexible_boxes , https://developer.mozilla.org/en-US/docs/Web/CSS/order , and https://developer.mozilla.org/en-US/docs/Web/CSS/flex )

The pref has been default-enabled for a while, so the removal isn't a behavior change (except for people who've explicitly opted to toggle it off for some reason).
Daniel: I just want to add on these very same compa notes that the pref is removed in Fx 28. To be sure that nobody try to toggle it off or is surprised because they did it long time ago.

Usually I just do this on the fly, but as I'm really late on the doc (other priorities!), I wanted to be sure to revisit it.
Whiteboard: [DocArea=CSS]
Whiteboard: [DocArea=CSS] → [DocArea=CSS][good first verify]
I've checked the docs and at it is correct, it doesn't imply that the pref has still an effect: ->dev-doc-complete
You need to log in before you can comment on or make changes to this bug.