Last Comment Bug 243412 - Implement 'box-sizing' (dropping the -moz- prefix)
: Implement 'box-sizing' (dropping the -moz- prefix)
Status: RESOLVED FIXED
[parity-IE] [parity-opera] [parity-we...
: css3, dev-doc-complete
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal with 64 votes (vote)
: mozilla29
Assigned To: Alex Henrie
:
Mentors:
http://www.w3.org/TR/css3-ui/#box-sizing
: 656562 892992 (view as bug list)
Depends on: 308801 338554 520992 761989 771414 776265 778413 801116 930218
Blocks: unprefix 852146 357457 850827 876050 946280
  Show dependency treegraph
 
Reported: 2004-05-12 08:05 PDT by Anne (:annevk)
Modified: 2014-03-28 00:31 PDT (History)
103 users (show)
ioana_damy: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
29+


Attachments
patch (untested) (3.01 KB, patch)
2004-05-12 08:08 PDT, Anne (:annevk)
no flags Details | Diff | Splinter Review
Patch v1 (6.04 KB, patch)
2013-04-06 08:37 PDT, :Ms2ger (⌚ UTC+1/+2)
dbaron: feedback+
Details | Diff | Splinter Review
box-sizing-tests (47.51 KB, patch)
2013-07-29 17:06 PDT, Scott Johnson (:jwir3)
no flags Details | Diff | Splinter Review
box-sizing-tests (74.50 KB, patch)
2013-07-30 10:21 PDT, Scott Johnson (:jwir3)
dbaron: review-
Details | Diff | Splinter Review
box-sizing-tests (83.00 KB, patch)
2013-09-30 09:59 PDT, Scott Johnson (:jwir3)
dbaron: review+
Details | Diff | Splinter Review
box-sizing-test Update (11.87 KB, patch)
2013-11-03 09:50 PST, Lukas Nordin
dbaron: review+
Details | Diff | Splinter Review
box-sizing-test Update (11.42 KB, patch)
2013-11-04 16:10 PST, Lukas Nordin
dbaron: review-
Details | Diff | Splinter Review
box-sizing-test-update2.patch (11.44 KB, patch)
2013-11-05 01:42 PST, Lukas Nordin
dbaron: review+
Details | Diff | Splinter Review
firefox-box-sizing.png (40.98 KB, image/png)
2013-12-29 19:35 PST, Damian Nowak
no flags Details
Enable unprefixed 'box-sizing' (3.71 KB, patch)
2014-01-17 00:04 PST, Alex Henrie
dbaron: review-
Details | Diff | Splinter Review
Bug 243412. Enable unprefixed 'box-sizing' (6.20 KB, patch)
2014-01-17 16:32 PST, Alex Henrie
dbaron: review+
dao+bmo: checkin+
Details | Diff | Splinter Review
Bug 243412. Use unprefixed box-sizing internally (144.85 KB, patch)
2014-01-26 18:36 PST, Alex Henrie
no flags Details | Diff | Splinter Review
Added box-sizing preference (1.49 KB, patch)
2014-02-03 15:06 PST, Lukas Nordin
no flags Details | Diff | Splinter Review
Added box-sizing preference (1.39 KB, patch)
2014-02-03 15:10 PST, Lukas Nordin
dbaron: review+
Details | Diff | Splinter Review
Add box-sizing preference (1.04 KB, patch)
2014-02-04 02:11 PST, Lukas Nordin
dbaron: review+
Details | Diff | Splinter Review

Description Anne (:annevk) 2004-05-12 08:05:09 PDT
CSS3 UI has reached CR. This means we can (and are encouraged to) drop the -moz-
prefix from the 'box-sizing' property.

Since quite a lot of sites rely on this property we should probably not remove
'-moz-box-sizing' but implement it like it was done with 'opacity'. (There is of
course the simple fact that lots of site authors were already using 'box-sizing'
because of Opera.)

Currently Mozilla supports 'padding-box' as a value, if we don't want to remove
that value, we should rename it to '-moz-padding-box', since it is not a valid
value according to the specification.
Comment 1 Anne (:annevk) 2004-05-12 08:08:07 PDT
Created attachment 148310 [details] [diff] [review]
patch (untested)

Since I don't have the ability to build and that will take some time before I
do I created a patch based on the 'moz-opacity' -> 'opacity' implementation.

It might be that I missed a few things, but like I said, I wasn't able to test
this.
Comment 2 Boris Zbarsky [:bz] 2004-05-12 08:44:54 PDT
1)  I don't see a reason to create a DOM alias.  box-sizing is not commonly
    scripted, unlike opacity.

2)  I'm rather opposed to throwing this switch unless some testing is done to
    see how good our implementation is.  In particular, I suspect that max and
    min widths are broken (especially on replaced elements).  In fact, I don't
    see how the CSS2.1 algorigthm for min/max widths on replaced elements makes
    sense as it stands in the presence of box-sizing.  I've sent mail to
    www-style to that effect.  In any case, someone needs to start writing
    testcases.
Comment 3 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2004-05-12 09:50:26 PDT
I suspect lots of stuff is broken and I'm opposed to the property in general.  I
think we should not implement it.
Comment 4 Hixie (not reading bugmail) 2004-05-12 10:15:48 PDT
Removing it altogether is fine by me.
Comment 5 Anne (:annevk) 2004-05-12 11:39:57 PDT
Although I don't really like the property either, the 'calc()' proposal
discussed on www-style seems much better, /this/ is the property people are
using. I visit some fora quite often and this is what people are using to "hack
around" the box model differences.

This is also one of the solutions people are talking about and which is placed
on various webdesigner/webdeveloper resources. We can of course say they are
nuts for using this stupid property, but I don't think they will understand and
I don't think that we should break sites that are already using it.

I think this needs to be implemented (we should at least keep the
'-moz-box-sizing') and to do that we probably have to wait for W3C clarification.
Comment 6 William J. Edney 2004-11-28 13:21:45 PST
I guess I'm one of the 'nuts'y people for wanting the ability to select between
box models, but the main reason why is that for the kind of web applications
that I'm building, 'content box' sizing is exactly the wrong kind of sizing I
need. I'm trying to do layouts that require 'border box' sizing. This is one
that Microsoft got right and the CSS committee got wrong.

In any case, there's no need to fight (or switch ;-)) - just give web developers
like me the choice via the 'box-sizing' property.

And *please*, if you're not going to implement this property, do *not* remove
-moz-box-sizing. Setting '* {-moz-box-sizing: border-box}' is the only way I've
been able to keep sane. My other stuff runs so well in Mozilla, but if I didn't
have border-box sizing, trying to lay out GUI would be a nightmare.

Cheers,

- Bill
Comment 7 Joël Kuiper 2005-04-08 05:23:18 PDT
It looks like box-sizing is dropped all together:
http://www.w3.org/TR/2002/WD-css3-box-20021024/#the-box-width
Comment 8 Anne (:annevk) 2005-04-08 05:26:11 PDT
Please note that you are referring to a very out-of-date draft, CSS3 UI which
included this property as mentioned in comment 0 is a bit more recent.
Comment 9 j.j. 2009-04-18 10:21:52 PDT
IE 8 supports "box-sizing" now without prefix (Opera anyhow since 7.0). This should increase importance.

It would be a mercy for web authors and web compatibility to get this fixed soon.
Comment 10 pikadudeno1+bugzilla 2010-03-03 14:00:21 PST
My use case for this, for anyone who's interested in hearing it:

I've got a DHTML-based puzzle game. Some of its UI is the same no matter where the player is the game, while the rest is in several "screens" that get dynamically switched between. Now, for this game, different screens need to look different depending on what they need to present to the player; screens may need paddings of varying sizes and one of them doesn't need a border. But they all need to be the same size.

Ideally, I would write
.screen {
/* dimensions go here */
box-sizing: border-box;
}
and Mission Accomplished.
Comment 11 Boris Zbarsky [:bz] 2010-03-03 14:10:00 PST
Sure, and right now you can write |-moz-box-sizing: border-box;|
Comment 12 pikadudeno1+bugzilla 2010-03-03 14:23:02 PST
Yes, as long as A. it doesn't get removed as per earlier discussion, and B. I repeat myself three times to ensure all major Web browsers listen to me. I'm a Web developer, not a leader of some snark-hunting expedition...
Comment 13 Lars Gunther 2010-03-25 06:29:28 PDT
1. What QA can one help out doing to get this fixed? 

2. Shouldn't Gecko perhaps drop the nonstandard padding-box value first?
Comment 14 j.j. 2010-11-08 19:18:01 PST
WebKit nightlies support box-sizing without -webkit-prefix now.
Comment 15 Florens Verschelde 2011-02-11 05:28:34 PST
Just pointing out that, in 2011, Firefox 4 (and possibly 5-6) will be the only browser left requiring a prefix for this well supported property.
Comment 16 Anthony Ricaud (:rik) 2011-02-11 06:41:14 PST
(In reply to comment #15)
> Just pointing out that, in 2011, Firefox 4 (and possibly 5-6) will be the only
> browser left requiring a prefix for this well supported property.
That's what the whiteboard is saying so it is known ;)

I think it's too late for Firefox 4 but can we get it triaged for Firefox 5 ?
Comment 17 Boris Zbarsky [:bz] 2011-02-11 08:38:27 PST
Florent, the property is not in CR.  It's not well-defined how it should behave in many cases in the spec yet.  Yes, some browsers dropped prefixes prematurely, but their implementations are not compatible with each other last I checked.  So I wouldn't call this "well supported"....
Comment 18 Boris Zbarsky [:bz] 2011-02-11 08:45:51 PST
And in particular, the interaction of box-sizing with replaced element sizing when max/min-width/height are used is still not defined.
Comment 19 Florens Verschelde 2011-02-11 15:11:57 PST
Thanks Boris for this background information. I hadn't tested the different implementations beyond display:block (non-replaced) elements. And I obviously don't advise dropping prefixes prematurely.
Comment 20 fantasai 2011-02-11 15:52:52 PST
Actually, it's been in CR for many years: css3-ui has been CR since 2004. The reason we haven't dropped prefixes is because there are some significant bugs with our implementation (see the dependencies).
Comment 21 Boris Zbarsky [:bz] 2011-02-11 18:16:29 PST
Er, you're right that CSS3 UI defines box-sizing.  But it doesn't define how it should work in enough detail that it can be implemented, leaving that to CSS3 Box, as far as I can tell...

But yes, we should fix the obvious dependent bugs, anyway, then create a decent test suite for this (which CSS3 UI certainly won't have otherwise).
Comment 22 Mats Palmgren (vacation) 2011-05-12 04:39:01 PDT
*** Bug 656562 has been marked as a duplicate of this bug. ***
Comment 23 Jan 2012-03-18 10:17:03 PDT
The relevant part is not dropping the -moz- but correcting the behavior of min-height and min-width, as for now these ignore box-sizing, while other browsers apply to min-xxx the same rules as to xxx.
Comment 24 Fredric Georgsson 2012-07-05 04:46:16 PDT
Only Firefox still uses the prefix.

http://caniuse.com/css3-boxsizing
Comment 25 Niloy Mondal 2012-07-23 05:20:56 PDT
Please drop the prefix already. You guys have dropped prefix for transitions & animations... duh.
Comment 26 Boris Zbarsky [:bz] 2012-07-23 06:10:48 PDT
Niloy, we dropped the prefix for those because their implementations match the spec.

The implementation of -moz-box-sizing does NOT match the spec.  See the bugs blocking this one.
Comment 27 russo 2012-07-25 08:52:04 PDT
i know the prefix was supposedly dropped, but it doesn't seem to be working without it. 2¢

with -moz prefix: desired result
http://jsbin.com/ezotow/6

without -moz prefix: broken
http://jsbin.com/ezotow/8
Comment 28 Paul Rouget [:paul] 2012-07-25 08:57:22 PDT
(In reply to russo from comment #27)
> i know the prefix was supposedly dropped

No. It has not been dropped yet (this bug is still open).
Comment 29 russo 2012-07-25 09:11:05 PDT
Apologies, thank you for the clarification
Comment 30 Félix Saparelli [:passcod] 2012-08-12 03:52:46 PDT
I'll give $50 NZD to anyone who fixes this bug before the end of 2012.
Comment 31 Niloy Mondal 2012-08-14 01:23:54 PDT
Lulz!

Seriously, please fix this, this property is turning out to be a little precious gem  and I have keep writing -moz-boz-sizing in my css.
Comment 32 fantasai 2012-08-14 09:40:31 PDT
If your comment is not about pinpointing the bug or contributing a fix, it is off-topic and does not belong here. These are our guidelines for using Bugzilla. Please read them before you add another comment:
  https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
Comment 33 Michał Gołębiowski [:m_gol] 2013-02-21 18:05:22 PST
There is one extremely important reason to drop the prefix even before fixing all issues with the current implementation - the technique of using:
* { box-sizing: border-box; }
has become popular recently (especially that some high-profile in web dev world people - as Paul Irish - has become advocating it) and if it's used for a mobile site, authors can easily forget about the prefix needed only for a browser with about 1% market share. And apart from most CSS3 features, not including a prefixed version of the property results not only in a minor degradation but in a completely broken site layout.

It would be interesting to know some stats about sites using border-box, especially on mobile. Nonetheless, this is something Mozilla should be wary about.
Comment 34 Chris Lord [:cwiiis] 2013-03-18 07:40:39 PDT
(In reply to Michał Z. Gołębiowski from comment #33)
> There is one extremely important reason to drop the prefix even before
> fixing all issues with the current implementation - the technique of using:
> * { box-sizing: border-box; }
> has become popular recently (especially that some high-profile in web dev
> world people - as Paul Irish - has become advocating it) and if it's used
> for a mobile site, authors can easily forget about the prefix needed only
> for a browser with about 1% market share. And apart from most CSS3 features,
> not including a prefixed version of the property results not only in a minor
> degradation but in a completely broken site layout.
> 
> It would be interesting to know some stats about sites using border-box,
> especially on mobile. Nonetheless, this is something Mozilla should be wary
> about.

This blocks bug 850827 - Google have started using an unprefixed box-sizing on the Google Maps mobile site. I agree with this comment, it'd be really great to have this unprefixed, even if there are problems.
Comment 35 Chris Lord [:cwiiis] 2013-03-18 08:04:13 PDT
Just to note, desktop GMail also uses box-sizing, but only with a -webkit prefix, meaning we get spurious horizontal scrollbars in the 3-panel mode :( (I'll file a bug for evangelism)
Comment 36 henryfhchan 2013-04-04 23:45:49 PDT
Bug 243412 does not depend on any open bugs now. Can it be unprefixed?
Comment 37 Chris Lord [:cwiiis] 2013-04-05 01:33:38 PDT
needinfo'ing :jwir3 wrt comment #36, as he expressed interest in this and it'd be good to get the ball rolling :)
Comment 38 Scott Johnson (:jwir3) 2013-04-05 14:16:13 PDT
(In reply to Chris Lord [:cwiiis] from comment #37)
> needinfo'ing :jwir3 wrt comment #36, as he expressed interest in this and
> it'd be good to get the ball rolling :)

Sure, I can look into this. Note that it's going to take a bit of time to determine the testing situation, and what we need to do to verify that our implementation matches spec. I will look into it in the next week or so.
Comment 39 :Ms2ger (⌚ UTC+1/+2) 2013-04-06 08:37:26 PDT
Created attachment 734251 [details] [diff] [review]
Patch v1

Not sure if anything else needs to be done, but this does the unprefixing itself.
Comment 40 Michał Gołębiowski [:m_gol] 2013-04-29 01:56:51 PDT
Any chance it'll get into Firefox 23? Does anything else have to be done?
Comment 41 Boris Zbarsky [:bz] 2013-04-29 09:30:39 PDT
> Any chance it'll get into Firefox 23?

Yes, if the review happens in the next week or so.

> Does anything else have to be done?

Well, running a nightly or better yet a try build with the patch through the box-sizing test suite (which exists, right?) would be a good start....
Comment 42 Michał Gołębiowski [:m_gol] 2013-05-08 08:32:12 PDT
OK, I've cloned the mozilla-inbound repo, build the OS X 64-bit debug build, run the test using:
./mach reftest layout/reftests/box-sizing/
then I applied the patch, re-run `./mach build` and then these tests.

All of them passed.

Anything else? :)
Comment 43 :Ms2ger (⌚ UTC+1/+2) 2013-05-08 08:41:47 PDT
(In reply to Boris Zbarsky (:bz) from comment #41)
> > Does anything else have to be done?
> 
> Well, running a nightly or better yet a try build with the patch through the
> box-sizing test suite (which exists, right?) would be a good start....

Looking in the csswg repo, I see no reason to believe that there is a test suite for any part of css-ui, fwiw.
Comment 44 Boris Zbarsky [:bz] 2013-05-08 09:10:32 PDT
> Anything else? :)

We obviously pass our own tests.

The question is whether our behavior sufficiently matches the spec to unprefix.

That requires either testing against the _spec_ test suite or verifying that our tests sufficiently match the spec and have sufficient coverage...
Comment 45 Scott Johnson (:jwir3) 2013-05-08 09:16:19 PDT
(In reply to :Ms2ger from comment #43)
> Looking in the csswg repo, I see no reason to believe that there is a test
> suite for any part of css-ui, fwiw.

(In reply to Boris Zbarsky (:bz) from comment #44)
> We obviously pass our own tests.
> 
> The question is whether our behavior sufficiently matches the spec to
> unprefix.
> 
> That requires either testing against the _spec_ test suite or verifying that
> our tests sufficiently match the spec and have sufficient coverage...

What Boris and Ms2ger are saying here is that there doesn't appear to be any tests in the CSS specification test suite, located here:
http://www.w3.org/Style/CSS/Test/

The css ui doesn't have any tests listed next to it, which indicates that there aren't any tests available for this spec yet. Probably as part of this bug, it might be a good idea to write some tests that should conform to the individual specification sections (see css3-multicol tests for an example). 

Take a look here for more information on writing tests, too:
http://wiki.csswg.org/test/format
Comment 46 Scott Johnson (:jwir3) 2013-05-08 09:17:30 PDT
So, basically, we need to:

A) Construct tests that identify what the spec requires the behavior of box-sizing to be,
B) Verify our implementation passes these tests, and
C) Submit the tests to the W3C.
Comment 47 Zlip792 2013-05-14 03:59:57 PDT
Hope dev will not mind this, since this patch passes FF own test suites and no W3C tests are available currently, I think real life testing by landing this patch in early FF24 Nightly cycle (which kicked in already) will be better if any site broken issues bugs filed against this, then it should be backout from next Aurora merger, all in all Nightly is bleeding edge test branch, so landing will not hurt. Just a suggestion..
Comment 48 Boris Zbarsky [:bz] 2013-05-14 06:47:43 PDT
The issue is not with whether sites are broken with the patch.

The issue is whether our implementation is broken enough that unprefixing it means we screw over web developers who then can't use the property.

Again, what needs to happen is checking what parts of the behavior are not tested and testing them.  As a simple example, there are apparently absolutely no tests that test the interaction of box-sizing and the treatment of max/min-width/height on replaced elements with intrinsic sizes (read: images) that have nonzero paddings and borders.
Comment 49 Zlip792 2013-05-14 08:36:01 PDT
Thanks boris for clearing up my confusion.. Keep up your work devs... I hope you don't mind my comment...
Comment 50 Robert Utasi [:hunboy] 2013-05-14 18:01:56 PDT
The computed height is wrong in padding|border|(margin)-box model with window.getComputedStyle(object,null).height function.
That currently doesn't follow the box model.
Current cross UA status: Chrome and Opera fits to box-model with this function.
Comment 51 j.j. 2013-05-15 06:23:29 PDT
(In reply to Robert Utasi [:hunboy] from comment #50)
> The computed height is wrong

Bug 520992? (fixed in Fx24)
Comment 52 Robert Utasi [:hunboy] 2013-05-15 10:27:16 PDT
(In reply to j.j. (mostly inactive in 2013, too) from comment #51)
> 
> Bug 520992? (fixed in Fx24)

Sorry for this bugspam, I've missed this codechange 1 month ago. I can confirm it works correctly on nightly with pagging|border-box model as well.
Comment 53 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-05-27 00:49:32 PDT
Comment on attachment 734251 [details] [diff] [review]
Patch v1

From a code perspective this patch looks fine, so r=dbaron on the code once we're ready to land it.  But clearing review request to indicate that it isn't yet ready to land.

However, before it lands, as Boris said, we could use some additional test coverage of box sizing -- in particular, for the image scaling code.  While the spec doesn't define precise wording, we should ensure that all the image scaling cases yield the same results for the same *specified content box width/height*, whether that width/height was specified directly or whether a larger width was specified and padding-box or border-box box-sizing was used.

I thought we had a bunch of tests for the image scaling code somewhere that could be modified to test this, but I can't find them.  That doesn't mean they aren't there, though.
Comment 54 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-06-19 22:08:35 PDT
I think the test I was thinking of was https://test.csswg.org/suites/css2.1/20110323/html4/replaced-min-max-001.htm

So a good start would be converting that to reftests (which I think may already be done somewhere), then adding variants with padding, border, and margin, and then adjusting those variants for different values of box-sizing to make sure that changes that don't change the specified content-box widths/heights don't change the size of the box.
Comment 55 Boris Zbarsky [:bz] 2013-07-12 11:42:06 PDT
*** Bug 892992 has been marked as a duplicate of this bug. ***
Comment 56 Allen Williams 2013-07-15 13:26:15 PDT
Is there any reason other than comments #3-5 why this bug can't be shipped in either 23 or 24?
There are no longer issues with this bug. There aren't many test cases. I suspect you cover over 99.9% of all cases with your implementation at least. Wouldn't it be easier to just ship it now and let people file bugs as the happen?
You already shipped the flexible box model in spite of the fact that the implementation is incomplete (IE flex-wrap:wrap). As a developer I can't stand vendor prefixes. This is the only prefixed property I use anymore (webkit included).
Comment 57 Boris Zbarsky [:bz] 2013-07-15 13:40:08 PDT
> Is there any reason other than comments #3-5 why this bug can't be shipped

Allen, please do read the whole bug; this is discussed at length starting in comment 44.
Comment 58 Allen Williams 2013-07-15 14:15:02 PDT
Believe me I have read the whole bug. I've been following this bug for 2 years at least.

I'm suggesting that any issues with the implementation are minor and it will be easier to find them by shipping now rather than trying desperately to create tests that will aim at the odd aspects of the spec. If you look at comment 45, he said that there doesn't seem to be any tests for this part of the spec.

The last couple of bugs that were blocking this were all minor. The image padding bug for example, I never encountered it and after I checked out that bug I had no problems creating a work around.

I only mentioned comments #3-5 because the very people that said they didn't want to implement this part of the spec are the same people that seem to be delaying it for looking for non-existent tests. In other words this is being delayed for political reasons rather than technical.
Comment 59 Boris Zbarsky [:bz] 2013-07-15 14:21:46 PDT
I think the issue is that for flexbox we knew in what ways it didn't match the spec and we agreed those were minor enough to ship.

The problem here is that we don't even know what's broken; once we do we can decide whether it's minor...
Comment 60 Allen Williams 2013-07-15 15:00:25 PDT
That's kind of my point. You don't know and you won't know what's broken.

If you were to ship now I can only see 3 things happening.

1) A website doesn't use box-sizing at all and shouldn't be affected
2) A website only uses non-prefixed box-sizing and is currently broken as a result (may or may not be fixed by shipping this patch)
3) A website uses the -moz- prefix in addition to being non-prefixed and either works or doesn't, but shipping this bug wouldn't change that.

A 4th scenario involves -moz- prefixing only, but that would only break if you removed the prefix rather than just not requiring it.

One commonality you'll notice in my scenarios is that anything that would be broken by shipping this is already broken anyway.

From what I've read here there are no tests and therefore this bug will continue to rot for another 10 years and you'll never know what is actually broken.

At the very least you should consider making a preference to enabling box-sizing unprefixed (personally I would rather skip that). That will at least allow us to actively search for websites that are broken under the implementation. As of right now there are websites that are broken by not shipping this patch as is, in fact you have 2 of them listed as blocked bugs. Maybe they have additional issues and maybe they don't, but you won't know until you ship this.
Comment 61 Boris Zbarsky [:bz] 2013-07-15 15:35:25 PDT
> 3) A website uses the -moz- prefix in addition to being non-prefixed and either works or
> doesn't, but shipping this bug wouldn't change that.

Assuming they use the same value for the two properties.

But the real issue with unprefixing broken crap is not site breakage today but causing issues for web developers tomorrow.

> there are no tests and therefore this bug will continue to rot 

No, what will happen is that someone will step up and write tests.  Worst case, the spec will need them anyway, so they will happen much sooner than 10 years from now.
Comment 62 Allen Williams 2013-07-15 15:56:56 PDT
Actually there are only 3 possible values. border-box, padding-box, and content-box. Firefox is the only vendor that supports padding-box. content-box is the default and is implemented automatically. That leaves border-box, so no that argument is invalid.

As a web developer myself I'm telling you right now you are causing more problems by NOT unprefixing it than the .0001% odd cases that MIGHT crop up as a result. The web developers of tomorrow are almost universally using it RIGHT NOW. Most of them develop on webkit. You are the ONLY vendor that doesn't support this. I can't tell you how many times I see webkit only prefixing (without the W3C version) in code. That's a problem in itself, but by eliminating as many vendor prefixes as possible, it minimizes the requirements to get things working in firefox. I'm also beginning to see people implement this as * { box-sizing:border-box; } (in other words firefox isn't supported) This is a MUCH bigger problem than a couple minor issues cropping up.

Why is it ok for IE (ironically I believe they were the first to support it), chrome, safari, and opera to do this a long time ago if they don't have any tests? It's obviously worked out for them. I can promise you that any odd issues that might crop up aren't worth the delay because you will fix more problems than you create.

The real problem you guys had was lack of support for min/max values as well as table support. That's been fixed for a while. One day out of the blue, the SVG bug disappeared, which was minor, as well as the image padding issue, which I had a work-around.
Comment 63 Scott Johnson (:jwir3) 2013-07-16 06:55:27 PDT
So, I've got some tests in my patch repository I've been working on. I should have them ready to post shortly. They might need some tweaking to get into W3C format. 

One thing I've noticed, though, is that webkit and opera don't seem to implement box-sizing for replaced elements. More specifically, the test in comment 54 that dbaron mentioned doesn't seem to be different with opera (I think I still have an old presto version) or chromium when I add padding to the images and then specify box-sizing: padding-box (or -o-box-sizing: padding-box or -webkit-box-sizing: padding-box).
Comment 64 Scott Johnson (:jwir3) 2013-07-16 06:57:14 PDT
(In reply to Scott Johnson (:jwir3) from comment #63)
> So, I've got some tests in my patch repository I've been working on. I
> should have them ready to post shortly. They might need some tweaking to get
> into W3C format. 
> 
> One thing I've noticed, though, is that webkit and opera don't seem to
> implement box-sizing for replaced elements. More specifically, the test in
> comment 54 that dbaron mentioned doesn't seem to be different with opera (I
> think I still have an old presto version) or chromium when I add padding to
> the images and then specify box-sizing: padding-box (or -o-box-sizing:
> padding-box or -webkit-box-sizing: padding-box).

Ah, you know, it might be just that webkit/presto don't implement padding-box...
Comment 65 philippe (part-time) 2013-07-16 07:25:54 PDT
(In reply to Scott Johnson (:jwir3) from comment #64)
 
> Ah, you know, it might be just that webkit/presto don't implement
> padding-box…

Afaict, IE 10 doesn't implement it either. If I remember correctly, 'padding-box' is a recent addition to the spec. And it is marked as 'at risk'.
Comment 66 Allen Williams 2013-07-16 07:56:55 PDT
Firefox appears to be the only browser that supports "padding-box". I wouldn't include it in any tests.
http://caniuse.com/#feat=css3-boxsizing
Comment 67 Scott Johnson (:jwir3) 2013-07-16 08:52:13 PDT
(In reply to Allen Williams from comment #66)
> Firefox appears to be the only browser that supports "padding-box". I
> wouldn't include it in any tests.
> http://caniuse.com/#feat=css3-boxsizing

We should probably have tests for these, though. Even if no browser pass the tests, it would be useful for future conformance.
Comment 68 David Bruant 2013-07-16 08:55:36 PDT
(In reply to Scott Johnson (:jwir3) from comment #67)
> (In reply to Allen Williams from comment #66)
> > Firefox appears to be the only browser that supports "padding-box". I
> > wouldn't include it in any tests.
> > http://caniuse.com/#feat=css3-boxsizing
> 
> We should probably have tests for these, though. Even if no browser pass the
> tests, it would be useful for future conformance.
Or if no other browser cares to implement it, maybe it should be removed from spec and Gecko... (let's have this discussion in a different place if interested)
Comment 69 Scott Johnson (:jwir3) 2013-07-29 17:06:20 PDT
Created attachment 782863 [details] [diff] [review]
box-sizing-tests

Finished tests for box-sizing, but there seems to be a slight problem on some platforms where the p element in box-sizing-replaced-{001,002}.html (by looking at it using the inspector) in example 1 below produces a box with a slightly larger height (~4px) than the <p> element in box-sizing-replaced-{001,002}.ref.html. I'm still analyzing why this is going wrong.

The devtools inspector shows that there are no margins or padding, and if I set the line-height to be the same on both <p> elements, or turn off margins and padding globally using the * selector, it has no effect. Basically, the problem comes up on OSX platforms - there's about a 4px gap between the top line and the bottom line (e.g. in https://tbpl.mozilla.org/?tree=Try&rev=2f1c02122c4f ) that isn't there in the reference images.
Comment 70 Scott Johnson (:jwir3) 2013-07-30 10:21:17 PDT
Created attachment 783214 [details] [diff] [review]
box-sizing-tests

Initial version of w3c box-sizing-tests.

https://tbpl.mozilla.org/?tree=Try&rev=a33150f7ef61
Comment 71 Lukas Nordin 2013-09-19 01:41:48 PDT
So what's needed to breath life into this bug again? Tests?
Comment 72 Allen Williams 2013-09-19 08:09:59 PDT
I still think if the people at mozilla didn't hate it, it would be implemented by now. Just look at the comments at the beginning of the page. I don't get the hate for it. And contrary to their comments I feel that while calc() is great, there are many cases where border-box is far superior. I use both where I feel appropriate. I just think its embarrassing that -moz-box-sizing is the only prefixed code I have left.

I may not like chrome, but I'm happy that they said they will never prefix anything else and plan to remove all prefixes. I just hope they actually kill them too. I don't ever want to see -webkit-border-radius again especially if its implemented. I wish Mozilla would adopt the no prefix policy as well.
Comment 73 Boris Zbarsky [:bz] 2013-09-19 08:25:40 PDT
> I wish Mozilla would adopt the no prefix policy as well.

We did, quite a while ago.
Comment 74 Boris Zbarsky [:bz] 2013-09-19 21:52:09 PDT
> So what's needed to breath life into this bug again? Tests?

A review from dbaron.
Comment 75 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-09-26 14:52:35 PDT
Comment on attachment 783214 [details] [diff] [review]
box-sizing-tests

In general, could you use "-ref" in filenames rather than ".ref"?
(Should be easy to fix by search-replace on the patch.)

The test files (but *not* the -ref files) need at least a <link
rel="help" pointing to the relevant section of the spec, which is
presumably http://www.w3.org/TR/css3-ui/#box-sizing .  A
<meta name="assert"> describing what the test is trying to test
wouldn't be a bad idea, either.

I think the box-sizing-content-box tests (not references) should use
percentages just like the box-sizing-padding-box and
box-sizing-border-box tests.  Otherwise they're just testing the initial
value of the property, which is tested well enough by
property_database.js-based tests.

box-sizing-padding-box-003.ref.xht (the reference!) probably should
have set widths/heights rather than using min-width/min-height, so
that it tests that min-width/height are working correctly a little
better.

box-sizing-replaced-* should probably mention in a comment at the
top that they're derived from replaced-min-max-001.

I don't understand why #img1 is so much simpler in your test than the
test you derived it from -- could you use the same test?

I'm also not convinced that the tests are all still testing what they
are supposed to at the new sizes.  My intent was that you'd still
produce a test that was leading to 75x75 images, but you'd increase the
numbers in the tests (which is using padding-box/border-box) to account
for the padding/border.  (Instead, you decreased the numbers in the
reference so all the images are smaller, but that means thet math that
was worked out to ensure all the combinations were tested isn't
*necessarily* valid anymore, though it might still be true.)  I think it
would be better to have 75x75 results, and increase the numbers in the
test file rather than decreasing them in the reference.  (And the
references should just be able to use width:75px and height:75px for
*all* the images rather than having min-width/max-width at all.)

I think these are good with those things fixed, but I'd like to take a
quick look at the box-sizing-replaced-* tests after those changes, so
marking as review-.  But thanks for writing these, and sorry for the
delay getting to the review.
Comment 76 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-09-26 14:55:34 PDT
Oh, and if you happen to have the replaced variant for content box sizing, that would be useful to have as a reftest if we don't already.
Comment 77 Scott Johnson (:jwir3) 2013-09-30 09:59:30 PDT
Created attachment 812123 [details] [diff] [review]
box-sizing-tests

(In reply to David Baron [:dbaron] (needinfo? me) from comment #75)
> Comment on attachment 783214 [details] [diff] [review]
> box-sizing-tests
> 
> In general, could you use "-ref" in filenames rather than ".ref"?
> (Should be easy to fix by search-replace on the patch.)

Fixed.

> The test files (but *not* the -ref files) need at least a <link
> rel="help" pointing to the relevant section of the spec, which is
> presumably http://www.w3.org/TR/css3-ui/#box-sizing .  A
> <meta name="assert"> describing what the test is trying to test
> wouldn't be a bad idea, either.

I did both of these, but you may want to review my wording for the <meta name="assert"> tags.

> I think the box-sizing-content-box tests (not references) should use
> percentages just like the box-sizing-padding-box and
> box-sizing-border-box tests.  Otherwise they're just testing the initial
> value of the property, which is tested well enough by
> property_database.js-based tests.

Done.

> box-sizing-padding-box-003.ref.xht (the reference!) probably should
> have set widths/heights rather than using min-width/min-height, so
> that it tests that min-width/height are working correctly a little
> better.

Fixed.

> box-sizing-replaced-* should probably mention in a comment at the
> top that they're derived from replaced-min-max-001.

Fixed, although, I noticed that the replaced-min-max-001 is flagged as having problems in the CSS test suite (#48 under "48 Untestable Reftests": http://wiki.csswg.org/test/reftest )

> I don't understand why #img1 is so much simpler in your test than the
> test you derived it from -- could you use the same test?

Yes, I modified this.

> I'm also not convinced that the tests are all still testing what they
> are supposed to at the new sizes.  My intent was that you'd still
> produce a test that was leading to 75x75 images, but you'd increase the
> numbers in the tests (which is using padding-box/border-box) to account
> for the padding/border.  (Instead, you decreased the numbers in the
> reference so all the images are smaller, but that means thet math that
> was worked out to ensure all the combinations were tested isn't
> *necessarily* valid anymore, though it might still be true.)  I think it
> would be better to have 75x75 results, and increase the numbers in the
> test file rather than decreasing them in the reference.  (And the
> references should just be able to use width:75px and height:75px for
> *all* the images rather than having min-width/max-width at all.)

Ok, I reworked all of the tests so that the height/width is adjusted such that it takes into account the padding/border in such a way that the image remains 75x75.

> I think these are good with those things fixed, but I'd like to take a
> quick look at the box-sizing-replaced-* tests after those changes, so
> marking as review-.  

Ok, I'm posting a new version now.

> But thanks for writing these, and sorry for the delay getting to the review.

You're welcome, and no problem regarding the delay.

(In reply to David Baron [:dbaron] (needinfo? me) from comment #76)
> Oh, and if you happen to have the replaced variant for content box sizing,
> that would be useful to have as a reftest if we don't already.

I added another test, box-sizing-replaced-003, which tests the box-sizing: content-box attribute. This is essentially the same as the replaced-min-max-001 that was in the w3c test suite. We have it in our tree under layout/reftests/bugs/234686-*.html, but these are individual tests rather than a larger set of 10 images in one page. I'm not sure if we want to duplicate the tests, so I added the new test for your perusal. Just let me know if you want to remove it. I guess it still tests to make sure that the -moz-box-sizing: content-box value is being processed correctly.
Comment 78 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-10-08 22:50:48 PDT
Comment on attachment 812123 [details] [diff] [review]
box-sizing-tests

(In reply to Scott Johnson (:jwir3) from comment #77)
> > The test files (but *not* the -ref files) need at least a <link
> > rel="help" pointing to the relevant section of the spec, which is
> > presumably http://www.w3.org/TR/css3-ui/#box-sizing .  A
> > <meta name="assert"> describing what the test is trying to test
> > wouldn't be a bad idea, either.
> 
> I did both of these, but you may want to review my wording for the <meta
> name="assert"> tags.


Instead of:

>+  <meta name="assert" content="The box-sized elements should each take up 1/2 of the horizontal space of the containing element, including their borders and padding." />

it would probably be better to write something like:

>+  <meta name="assert" content="-moz-box-sizing: border-box should make the element's (percentage) width be the distance from the left border edge to the right border edge." />

and similar for the other tests.  In other words, describe the feature
being tested rather than exactly what the test shows.


In box-sizing-replaced-001.xht, change:

  #img1 max-width from 175px to 115px
  #img2 min-height from 60px to 70px

unless I'm missing a reason they should be as they are.

same for box-sizing-replaced-002:
  #img1 max-width from 185px to 125px
  #img2 min-height from 60px to 80px

same for box-sizing-replaced-003:
  #img1 max-width from 165px to 125px
  #img2 min-height from 50px to 60px

r=dbaron with that
Comment 79 Jet Villegas (:jet) 2013-10-15 11:44:37 PDT
I can push these two patches into the tree if they're ready to go. Can I assume the feedback+ is now an r+ if the tests land as described in Comment 78?
Comment 80 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-10-15 16:49:54 PDT
Yes.
Comment 81 mkstix6 2013-10-22 03:06:27 PDT
Just a word of encouragement guys. I don't do anything particularly complicated replacing elements (reading this thread briefly it seems to be the sticking point as I understand it).

But.
I've been using:

-moz-box-sizing:border-box;
box-sizing:border-box;

Without issue, for months and months now. Love this property for various reasons e.g. for making input and select the same width in a responsive design, it is saving me plenty of time and my sanity.

Keep up the good work, I hope we can resolve the final issues and drop the prefix soon.
The only thing I want more from FireFox is flex-wrap.
Comment 82 Boris Zbarsky [:bz] 2013-10-23 18:07:41 PDT
Bug 930218 seems to be an obvious issue in the box-sizing implementation...
Comment 83 Andrew Truong [:feer56] 2013-11-01 08:55:20 PDT
Just some twitter reports: https://twitter.com/drewtru/status/396301962192224256
Comment 84 Lukas Nordin 2013-11-03 09:50:43 PST
Created attachment 826471 [details] [diff] [review]
box-sizing-test Update

This patch makes the changes noted in comment #78. 
It uses the exact wording you made, with just padding, border, and content words switched out depending on the test.

This patch should be applied on top of Scotts :)
Comment 85 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-11-04 15:36:49 PST
Comment on attachment 826471 [details] [diff] [review]
box-sizing-test Update

>diff -r c2ef3c2742f8 -r 2c04e4858e1e layout/reftests/w3c-css/submitted/ui3/box-sizing-border-box-003.xht
>+  <meta name="assert" content="-moz-box-sizing: border-box should make the element's (percentage) width be the distance from the left border edge to the right border edge." />

border-box-003 tests both width and height

>diff -r c2ef3c2742f8 -r 2c04e4858e1e layout/reftests/w3c-css/submitted/ui3/box-sizing-border-box-004.xht
>+  <meta name="assert" content="-moz-box-sizing: border-box should make the element's (percentage) width be the distance from the left border edge to the right border edge." />

border-box-004 tests min/max-width/height, and uses lengths rather than percentages

>diff -r c2ef3c2742f8 -r 2c04e4858e1e layout/reftests/w3c-css/submitted/ui3/box-sizing-content-box-001.xht
>+  <meta name="assert" content="-moz-box-sizing: content-box should make the element's (percentage) width be the distance from the left content edge to the right content edge."/>

This tests both width and height.

>diff -r c2ef3c2742f8 -r 2c04e4858e1e layout/reftests/w3c-css/submitted/ui3/box-sizing-content-box-002.xht
>+  <meta name="assert" content="-moz-box-sizing: content-box should make the element's (percentage) width be the distance from the left content edge to the right content edge."/>

This tests calc() with lengths and percentages, not plain percentages, and tests both width and height.

>diff -r c2ef3c2742f8 -r 2c04e4858e1e layout/reftests/w3c-css/submitted/ui3/box-sizing-content-box-003.xht
>+  <meta name="assert" content="-moz-box-sizing: content-box should make the element's (percentage) width be the distance from the left content edge to the right content edge."/>

This tests both width and height.

>diff -r c2ef3c2742f8 -r 2c04e4858e1e layout/reftests/w3c-css/submitted/ui3/box-sizing-padding-box-002.xht
>+  <meta name="assert" content="-moz-box-sizing: padding-box should make the element's (percentage) width be the distance from the left padding edge to the right padding edge."/>

This tests both width and height.

>diff -r c2ef3c2742f8 -r 2c04e4858e1e layout/reftests/w3c-css/submitted/ui3/box-sizing-padding-box-003.xht
>+  <meta name="assert" content="-moz-box-sizing: padding-box should make the element's (percentage) width be the distance from the left padding edge to the right padding edge."/>

This tests calc() with lengths and percentages, not plain percentages, and tests both width and height.


r=dbaron with those corrections
Comment 86 Lukas Nordin 2013-11-04 16:10:57 PST
Created attachment 827085 [details] [diff] [review]
box-sizing-test Update

> This tests calc() with lengths and percentages, not plain percentages, and tests both width and height.
So just "element's width"?

> border-box-004 tests min/max-width/height, and uses lengths rather than percentages
It would be pixels, right?

Also, shouldn't the tests use box-sizing instead of -moz-box-sizing?
Comment 87 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-11-04 18:19:14 PST
Comment on attachment 827085 [details] [diff] [review]
box-sizing-test Update

(In reply to Lukas Nordin from comment #86)
> Created attachment 827085 [details] [diff] [review]
> box-sizing-test Update
> 
> > This tests calc() with lengths and percentages, not plain percentages, and tests both width and height.
> So just "element's width"?

"element's (calc) width and height"

> > border-box-004 tests min/max-width/height, and uses lengths rather than percentages
> It would be pixels, right?

pixels are a type of length; the point here is that we have a length test and a percentage test, so it's better to say length

> Also, shouldn't the tests use box-sizing instead of -moz-box-sizing?

not in this patch
Comment 88 Lukas Nordin 2013-11-05 01:42:29 PST
Created attachment 827301 [details] [diff] [review]
box-sizing-test-update2.patch

> "element's (calc) width and height"
Done!

> pixels are a type of length; the point here is that we have a length test and a percentage test, so it's better to say length
Got it!
Comment 89 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-11-05 23:10:35 PST
Comment on attachment 827301 [details] [diff] [review]
box-sizing-test-update2.patch

r=dbaron
Comment 90 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-11-05 23:27:00 PST
Landed the tests:

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/c3501cfab6b5
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/1525f72e55ea

Still need to:
 (1) land the enabling
 (2) prefix-substitute the tests
 (3) push the tests to the w3c repository
Comment 92 Michał Gołębiowski [:m_gol] 2013-11-23 16:16:54 PST
(In reply to David Baron [:dbaron] (needinfo? me) (UTC+8) from comment #90)
> Still need to:
>  (1) land the enabling

What do you mean by "land the enabling"?

>  (2) prefix-substitute the tests

I assume this is a simple thing to do now that tests are written?

>  (3) push the tests to the w3c repository

Are there any technical problems concerning that?

I'd like to see box-sizing unprefixed soon so I wonder if there are any remaining technical problems for that to happen.
Comment 93 Michał Gołębiowski [:m_gol] 2013-12-10 09:23:22 PST
A basic search on GitHub:
https://github.com/search?q=box-sizing%3A+border-box&ref=cmdform&type=Code
shows that `box-sizing: border-box` is used very often and very few people add a `-moz-` prefix to the list.

Now that all the hard work has been done (thanks a lot for that!), there are no hanging critical bugs, tests have been written etc., can we get this into Firefox 29?
Comment 94 Lukas Nordin 2013-12-18 17:28:11 PST
(In reply to Michał Gołębiowski from comment #92)
> (In reply to David Baron [:dbaron] (needinfo? me) (UTC+8) from comment #90)
> > Still need to:
> >  (1) land the enabling
> 
> What do you mean by "land the enabling"?
> 
> >  (2) prefix-substitute the tests
> 
> I assume this is a simple thing to do now that tests are written?
> 
> >  (3) push the tests to the w3c repository
> 
> Are there any technical problems concerning that?
> 
> I'd like to see box-sizing unprefixed soon so I wonder if there are any
> remaining technical problems for that to happen.

Any comment on this?
Comment 95 Damian Nowak 2013-12-29 19:35:45 PST
Created attachment 8351744 [details]
firefox-box-sizing.png

Ridiculous.
Comment 96 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2013-12-29 19:49:15 PST
(In reply to Michał Gołębiowski from comment #92)
> (In reply to David Baron [:dbaron] (needinfo? me) (UTC+8) from comment #90)
> > Still need to:
> >  (1) land the enabling
> 
> What do you mean by "land the enabling"?

Write and land a patch to enable unprefixed 'box-sizing'.

> >  (2) prefix-substitute the tests
> 
> I assume this is a simple thing to do now that tests are written?

Yes.

> >  (3) push the tests to the w3c repository
> 
> Are there any technical problems concerning that?

I don't know.  At a minimum we'd probably want to move them to the layout/reftests/w3c-css/submitted/ directory.  But there might be a bunch of other issues raised by the code that checks them when they get submitted to the w3c repository.
Comment 97 Alex Henrie 2014-01-17 00:04:39 PST
Created attachment 8361541 [details] [diff] [review]
Enable unprefixed 'box-sizing'

I've rebased Ms2ger's patch to enable unprefixed 'box-sizing'. See comment #39 and comment #53.
Comment 98 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-01-17 16:05:32 PST
Comment on attachment 8361541 [details] [diff] [review]
Enable unprefixed 'box-sizing'

The change you removed from the original patch needs to go in nsComputedDOMStylePropertyList.h, but otherwise this looks fine.
Comment 99 Alex Henrie 2014-01-17 16:32:29 PST
Created attachment 8361992 [details] [diff] [review]
Bug 243412. Enable unprefixed 'box-sizing'

Thanks for catching that. This should do the trick.
Comment 100 Daniel Holbert [:dholbert] (mostly OOTO until Aug 9th) 2014-01-24 11:47:44 PST
Since you're using checkin-needed -- could you indicate which of the patches here need checkin?  Looks like there are 4 different patches attached, one with feedback+ and 3 with review+. Do the last 3 need landing?
Comment 101 Alex Henrie 2014-01-24 12:04:14 PST
Just attachment 8361992 [details] [diff] [review]. I'll make sure to specify next time.
Comment 102 Dão Gottwald [:dao] 2014-01-25 08:16:53 PST
Comment on attachment 8361992 [details] [diff] [review]
Bug 243412. Enable unprefixed 'box-sizing'

https://hg.mozilla.org/integration/mozilla-inbound/rev/36dfbfb486f5
Comment 103 Phil Ringnalda (:philor, back in August) 2014-01-25 19:58:06 PST
https://hg.mozilla.org/mozilla-central/rev/36dfbfb486f5
Comment 104 Florian Bender 2014-01-26 04:14:37 PST
So I assume the last push fixed this, and there's nothing left to land (e.g. tests)?
Comment 105 Alex Henrie 2014-01-26 18:36:23 PST
Created attachment 8365750 [details] [diff] [review]
Bug 243412. Use unprefixed box-sizing internally

David Baron (comment #90) said we need to:
>  (1) land the enabling
>  (2) prefix-substitute the tests
>  (3) push the tests to the w3c repository

The last patch was step 1. This patch takes care of step 2. However, I'm not sure that I was supposed to change files in the following directories:

browser/extensions/pdfjs
layout/reftests/w3c-css/submitted
testing/xpcshell/node-http2

At any rate, it should be easy to tweak the patch as needed.
Comment 107 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-01-31 12:56:35 PST
(In reply to Alex Henrie from comment #105)
> Created attachment 8365750 [details] [diff] [review]
> Bug 243412. Use unprefixed box-sizing internally
> 
> David Baron (comment #90) said we need to:
> >  (1) land the enabling
> >  (2) prefix-substitute the tests
> >  (3) push the tests to the w3c repository

I realized I missed one bit:

  (1b) add a preference like those added in bug 804944

> The last patch was step 1. This patch takes care of step 2. However, I'm not
> sure that I was supposed to change files in the following directories:
> 
> browser/extensions/pdfjs
> layout/reftests/w3c-css/submitted
> testing/xpcshell/node-http2
> 
> At any rate, it should be easy to tweak the patch as needed.

The tests in layout/reftests/w3c-css/submitted were actually the primary subject of "the tests" I was talking about in part 2.

The other patches are good, but it's probably worth splitting them into multiple patches for different reviewers to look at.  In particular, I'd split them up as:

 * browser/extensions/pdfjs (r? to :bdahl)
 * rest of browser/ and toolkit/ (r? to :gavin or similar... who might ask for it to be split further)
 * mobile/android/ (r? to :mfinkle or :margaret or similar)
 * content, dom, layout, widget (r? to me)

and skip these changes, which shouldn't be touched since they're imported code:
 * testing/xpcshell/node-http2/

It's probably worth filing a separate bug for this, and marking this bug as fixed.
Comment 108 Allen Williams 2014-01-31 14:45:44 PST
If you do make this a preference setting, please consider defaulting it to enabled.
Comment 109 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-01-31 15:26:07 PST
The preference would be a preference for the prefixed -moz-box-sizing (not the unprefixed box-sizing), defaulting to enabled, so that authors can test their sites in advance of our dropping support for the prefixed version.  See http://dbaron.org/log/20130225-removing-prefixes
Comment 110 Lukas Nordin 2014-02-01 06:36:06 PST
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #109)
> The preference would be a preference for the prefixed -moz-box-sizing (not
> the unprefixed box-sizing), defaulting to enabled, so that authors can test
> their sites in advance of our dropping support for the prefixed version. 
> See http://dbaron.org/log/20130225-removing-prefixes

What would be the steps for adding this?

    CSS_PROP_ALIAS(-moz-box-sizing, box_sizing, MozBoxSizing, "layout.css.prefixes.boxsizing")
and
    pref("layout.css.prefixes.boxsizing", true);
?
Comment 111 Boris Zbarsky [:bz] 2014-02-03 07:26:16 PST
Maybe "layout.css.prefixes.box-sizing" for the pref name.  Other than that, comment 110 is exactly right.
Comment 112 Lukas Nordin 2014-02-03 15:06:00 PST
Created attachment 8369703 [details] [diff] [review]
Added box-sizing preference
Comment 113 Lukas Nordin 2014-02-03 15:10:01 PST
Created attachment 8369709 [details] [diff] [review]
Added box-sizing preference

Well... I did a stupid, sorry about that. This one should do it!
Comment 114 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-02-03 15:45:09 PST
Comment on attachment 8369709 [details] [diff] [review]
Added box-sizing preference

># HG changeset patch
># User Lukas Nordin <lukasnordin11@gmail.com>
>Bug 243412: Added box-sizing preference to control the vendor prefix; r=dbaron

Use "Add" in the commit message rather than "Added".

>diff --git a/layout/style/nsCSSPropList.h b/layout/style/nsCSSPropList.h
>--- a/layout/style/nsCSSPropList.h
>+++ b/layout/style/nsCSSPropList.h
>@@ -1384,7 +1384,7 @@
>     box_sizing,
>     BoxSizing,
>     CSS_PROPERTY_PARSE_VALUE,
>-    "",
>+    "layout.css.prefixes.box-sizing",
>     VARIANT_HK,
>     kBoxSizingKTable,
>     CSS_PROP_NO_OFFSET,

You don't want this; this puts the main property behind a pref!

r=dbaron on the other two changes
Comment 115 Lukas Nordin 2014-02-04 02:11:48 PST
Created attachment 8369944 [details] [diff] [review]
Add box-sizing preference

> Use "Add" in the commit message rather than "Added".
Ok!
> You don't want this; this puts the main property behind a pref!
Done.
Comment 116 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-02-04 14:53:20 PST
Comment on attachment 8369944 [details] [diff] [review]
Add box-sizing preference

When you get review+ with specific changes, and you make those changes, you don't need to ask for review again.

But r=dbaron again anyway.
Comment 117 Florian Bender 2014-02-04 15:03:22 PST
Check-in needed for attachment #8369944 [details] [diff] [review]. 

(Should this request be using the checkin? flag on the attachment?)

Needs uplift as well because the original patch landed in train 29.
Comment 118 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-02-04 18:08:06 PST
I landed the above patch and the prefix-substituting of the tests:
   https://hg.mozilla.org/integration/mozilla-inbound/rev/75ac8af8c495
   https://hg.mozilla.org/integration/mozilla-inbound/rev/13f6dad08f3a

We should probably close this bug and do the substitution of our own code in other bugs (i.e., continue from attachment 836570).
Comment 119 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-02-04 18:28:36 PST
And test updates imported in http://hg.csswg.org/test/rev/2aa42c3a4da5
Comment 120 Boris Zbarsky [:bz] 2014-02-04 18:53:35 PST
I'm not sure we really _need_ uplift for the preference thing.  Not uplifting it just means that in 29 both the prefixed and unprefixed form will be supported unconditionally, which is not fatal.  Starting with 30 people will be able to experiment with the removal of the prefixed form.  So all it'll really do is delay when we can remove the prefixed form by that one release cycle.

On the other hand, uplifting that patch won't hurt anything either; it's pretty simple...
Comment 121 Alex Henrie 2014-02-04 20:53:54 PST
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-8) from comment #118)
> We should probably close this bug and do the substitution of our own code in
> other bugs (i.e., continue from attachment 836570).

I've created bug 968029 for this purpose.
Comment 122 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-02-04 21:02:12 PST
Comment on attachment 8365750 [details] [diff] [review]
Bug 243412. Use unprefixed box-sizing internally

cancelling this review request since this is now happening in bug 968029 (as described in comment 107)
Comment 123 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-02-04 22:27:27 PST
It's probably better not to uplift the preference, since if anyone tried it, the browser would end up somewhat broken (lacking the fixes in bug 968029).
Comment 124 Boris Zbarsky [:bz] 2014-02-05 00:01:59 PST
We should probably just resolve this as FIXED, then.
Comment 125 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2014-02-05 00:58:55 PST
Yep.

Also setting assignee to one of the many people who helped get this code in.
Comment 160 Ioana (away) 2014-03-28 00:31:52 PDT
It seems this fix is already covered automatically:
https://hg.mozilla.org/mozilla-central/rev/13f6dad08f3a

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