801098 - Unprefix CSS3 flexbox

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Daniel Holbert [:dholbert]]

Just talked to dbaron, and we agree that while the last flexbox implementation issues are being hashed out, it's fine to have it unprefixed, as long as it's still preffed off by default.

So: filing this bug on unprefixing all the CSS3 flexbox properties/keywords.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: (wrong patch file, ignore)

I'm splitting this into 3 parts:
 - changes in code
 - changes in mochitests
 - changes in reftests/crashtests
...though I'll fold the three together when landing, because they need to all land together to keep from breaking tests.

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: part 1: unprefix in code

(oops, here's the right patch)

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: part 2: unprefix in mochitests

(This also unprefixes some -moz-calc and -moz-keyframes usages -- those are there because I originally wrote those test files before we unprefixed those keywords, and landed them after we unprefixed)

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: part 3: unprefix in reftests/crashtests

Note: the first chunk of this patch fixes up the crashtest for bug 737313, which was added back when "-moz-flexbox" was the display keyword.

This patch replaces that with "display: flex", and also adds the pref() annotation to it in the crashtest manifest while I'm at it.

Comment 5

[Daniel Holbert [:dholbert]]

Try run w/ just these patches:
   https://tbpl.mozilla.org/?tree=Try&rev=514f0370ce25
Try run w/ these patches, plus a patch to enable the flexbox pref by default (which enables a few more tests):
  https://tbpl.mozilla.org/?tree=Try&rev=e78ce92b3c73

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #5)
> Try run w/ these patches, plus a patch to enable the flexbox pref by default
> (which enables a few more tests):
>   https://tbpl.mozilla.org/?tree=Try&rev=e78ce92b3c73

That one had some orange due to mochitests/reftests expectation about the pref being disabled. Changed those expectations and pushed again, for a greener Try run:
  https://tbpl.mozilla.org/?tree=Try&rev=576bb4e670ff

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 671044 [details] [diff] [review]
part 1: unprefix in code

r=dbaron, but I hope these nsIDOMCSS2Properties.idl changes aren't detectable thanks to our use of the new bindings.  (Can we remove that file yet?!)

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 671046 [details] [diff] [review]
part 3: unprefix in reftests/crashtests

Interesting that a few tests had -moz-flexbox instead of -moz-flex.

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to David Baron [:dbaron] from comment #8)
> Interesting that a few tests had -moz-flexbox instead of -moz-flex.

Yeah, just a couple crashtests added back when that was the keyword. (see first chunk of comment 4.)

Thanks for the review!

Comment 10

[Daniel Holbert [:dholbert]]

(In reply to David Baron [:dbaron] from comment #7)
> r=dbaron, but I hope these nsIDOMCSS2Properties.idl changes aren't
> detectable thanks to our use of the new bindings.  (Can we remove that file
> yet?!)

I think we're still using that file for something, but I don't remember what.  (I think bz knows, though...)

I realized that I forgot to rev the UUID of that interface, though (for the benefit of whatever straggling code still uses that file :)  )  I fixed that locally, folded the patches together per comment 1, and pushed:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/8a070a22c369

Comment 11

[Boris Zbarsky [:bzbarsky]]

> I think we're still using that file for something, but I don't remember what.

We only use it if the CSSDeclaration binding is preffed off.

I'll work on removing that pref and that IDL file.  Bug 801819.

Comment 12

[Kang-Hao (Kenny) Lu [:kennyluck]]

Would this land by Friday? I am supposed to lead the effort to write css3-flex tests in this weekends' Test the Web Forward @ Beijing and it would of course be nice to reduce as much trouble as we can. :)

Comment 13

[Daniel Holbert [:dholbert]]

Thanks Kenny -- I actually just landed it on inbound, in comment 10!  If all goes well, it should be merged to mozilla-central within the next day or so.

Comment 14

[Kang-Hao (Kenny) Lu [:kennyluck]]

(In reply to Daniel Holbert [:dholbert] from comment #13)
> Thanks Kenny 

*We* should all thank you :)

> -- I actually just landed it on inbound, in comment 10!  If all
> goes well, it should be merged to mozilla-central within the next day or so.

Congrats!

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/8a070a22c369

Comment 16

[Jean-Yves Perrier [:teoli]]

Documented (without speaking about the prefix): https://bugzilla.mozilla.org/show_bug.cgi?id=666041#c132

Comment 17

[Daniel Holbert [:dholbert]]

This might be worth mentioning in a release note for Firefox 19.

(This feature is preffed-off, just like it was in Firefox 18 -- but for users who've manually preffed it on, the css3 flexbox CSS properties / keywords no longer use a vendor-prefix.)

Comment 18

[Alex Keybl [:akeybl]]

(In reply to Daniel Holbert [:dholbert] from comment #17)
> This might be worth mentioning in a release note for Firefox 19.
> 
> (This feature is preffed-off, just like it was in Firefox 18 -- but for
> users who've manually preffed it on, the css3 flexbox CSS properties /
> keywords no longer use a vendor-prefix.)

We haven't relnote'd disabled features in the past.