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

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Daniel Holbert [:dholbert]]

(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.)

Comment 2

[Daniel Holbert [:dholbert]]

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.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 1: remove flexbox pref from reftests/crashtests

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.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 2: Adjust non-flexbox-specific mochitests to no longer bother with flexbox pref

This patch removes flexbox-pref-specific stuff from some "all-property"-ish mochitests.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: part 3: Remove pref-setting mochitest wrappers for flexbox-specific mochitests, and transfer their boilerplate into the actual test files (currently named "file_")

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.)

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: part 4: rename the now-fixed-up file_XYZ mochitests to test_XYZ.

Here's the next part, to rename the file_* files to test_*.

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: part 5: remove C++ code that deals with the pref

Comment 8

[Daniel Holbert [:dholbert]]

Attachment: part 6: remove pref from all.js

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.)

Comment 9

[Daniel Holbert [:dholbert]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=eb73605597de

Comment 10

[Mats Palmgren (inactive)]

Comment on attachment 829700 [details] [diff] [review]
part 6: remove pref from all.js

r=mats  Thanks for the helpful comments.

Comment 11

[Daniel Holbert [:dholbert]]

Thanks! Landed:
part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/3a3b8317394b
part 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/97bda1580066
part 3: https://hg.mozilla.org/integration/mozilla-inbound/rev/a4ff3c828a73
part 4: https://hg.mozilla.org/integration/mozilla-inbound/rev/598570ccfca5
part 5: https://hg.mozilla.org/integration/mozilla-inbound/rev/1a09d295aa1c
part 6: https://hg.mozilla.org/integration/mozilla-inbound/rev/c106cbe8457e

Comment 12

[Daniel Holbert [:dholbert]]

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).

Comment 13

[Jean-Yves Perrier [:teoli]]

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.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3a3b8317394b
https://hg.mozilla.org/mozilla-central/rev/97bda1580066
https://hg.mozilla.org/mozilla-central/rev/a4ff3c828a73
https://hg.mozilla.org/mozilla-central/rev/598570ccfca5
https://hg.mozilla.org/mozilla-central/rev/1a09d295aa1c
https://hg.mozilla.org/mozilla-central/rev/c106cbe8457e

Comment 15

[Jean-Yves Perrier [:teoli]]

I've checked the docs and at it is correct, it doesn't imply that the pref has still an effect: ->dev-doc-complete