Last Comment Bug 801098 - Unprefix CSS3 flexbox
: Unprefix CSS3 flexbox
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla19
Assigned To: Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th)
:
Mentors:
Depends on: 796212
Blocks: unprefix
  Show dependency treegraph
 
Reported: 2012-10-12 14:23 PDT by Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th)
Modified: 2013-02-22 08:28 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
(wrong patch file, ignore) (1.89 KB, patch)
2012-10-13 00:01 PDT, Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th)
no flags Details | Diff | Splinter Review
part 1: unprefix in code (23.92 KB, patch)
2012-10-13 00:02 PDT, Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th)
dbaron: review+
Details | Diff | Splinter Review
part 2: unprefix in mochitests (56.59 KB, patch)
2012-10-13 00:05 PDT, Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th)
dbaron: review+
Details | Diff | Splinter Review
part 3: unprefix in reftests/crashtests (236.46 KB, patch)
2012-10-13 00:08 PDT, Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th)
dbaron: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th) 2012-10-12 14:23:47 PDT
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] (mostly OOTO until Aug 9th) 2012-10-13 00:01:22 PDT
Created attachment 671043 [details] [diff] [review]
(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] (mostly OOTO until Aug 9th) 2012-10-13 00:02:41 PDT
Created attachment 671044 [details] [diff] [review]
part 1: unprefix in code

(oops, here's the right patch)
Comment 3 Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th) 2012-10-13 00:05:42 PDT
Created attachment 671045 [details] [diff] [review]
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] (mostly OOTO until Aug 9th) 2012-10-13 00:08:38 PDT
Created attachment 671046 [details] [diff] [review]
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] (mostly OOTO until Aug 9th) 2012-10-13 00:15:57 PDT
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] (mostly OOTO until Aug 9th) 2012-10-13 10:21:12 PDT
(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+2 (mostly busy through August 4; review requests must explain patch) 2012-10-15 11:52:45 PDT
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+2 (mostly busy through August 4; review requests must explain patch) 2012-10-15 11:56:43 PDT
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] (mostly OOTO until Aug 9th) 2012-10-15 12:10:35 PDT
(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] (mostly OOTO until Aug 9th) 2012-10-15 12:44:17 PDT
(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 [:bz] 2012-10-15 13:28:00 PDT
> 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] 2012-10-15 13:52:14 PDT
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] (mostly OOTO until Aug 9th) 2012-10-15 13:58:10 PDT
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] 2012-10-15 14:04:20 PDT
(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] 2012-10-16 01:23:20 PDT
https://hg.mozilla.org/mozilla-central/rev/8a070a22c369
Comment 16 Jean-Yves Perrier [:teoli] 2012-12-08 11:54:08 PST
Documented (without speaking about the prefix): https://bugzilla.mozilla.org/show_bug.cgi?id=666041#c132
Comment 17 Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th) 2013-02-15 15:42:30 PST
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] 2013-02-22 08:28:27 PST
(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.

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