Last Comment Bug 823483 - Percentage max-width does not seem to affect contributions to intrinsic min-width
: Percentage max-width does not seem to affect contributions to intrinsic min-w...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout: Block and Inline (show other bugs)
: Trunk
: All All
: -- normal with 4 votes (vote)
: mozilla47
Assigned To: David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
:
Mentors:
: 1139931 1215103 1285906 (view as bug list)
Depends on: 504622 1247897 1247929 1248174 1248749
Blocks: 822285 1170774 332171 434230 441046 932996 936088 975632 980432 1023118 1047886 1060131 1085106 1215103 1227140 1238900 1244192
  Show dependency treegraph
 
Reported: 2012-12-20 04:59 PST by Tim Hunt
Modified: 2016-07-14 13:12 PDT (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
affected
verified
verified

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
workaround.htm (668 bytes, text/html)
2015-03-23 15:08 PDT, Francesco
no flags Details
some tests with <img> (1.69 KB, text/html)
2015-11-20 15:11 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
no flags Details
some tests with empty <input type="button"> (1.62 KB, text/html)
2015-11-20 15:16 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
no flags Details
some tests with <input type="button"> (1.82 KB, text/html)
2015-11-20 15:19 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
no flags Details
First testcase rendered in Edge (10.09 KB, image/png)
2015-11-20 16:28 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
no flags Details
Second testcase rendered in Edge (10.77 KB, image/png)
2015-11-20 16:28 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
no flags Details
Third testcase rendered in Edge (12.82 KB, image/png)
2015-11-20 16:29 PST, Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary)
no flags Details
img-testcase.html (2.17 KB, text/html)
2015-12-04 04:47 PST, percyley
no flags Details
img-testcase-Firefox.png (178.31 KB, image/png)
2015-12-04 04:48 PST, percyley
no flags Details
img-testcase-Chrome.png (56.97 KB, image/png)
2015-12-04 04:48 PST, percyley
no flags Details
MozReview Request: Bug 823483 patch 1 - Check for percentage max-width in addition to percentage width when deciding to ignore intrinsic min-width of replaced elements. r?dholbert (58 bytes, text/x-review-board-request)
2016-01-29 19:16 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
dholbert: review+
lhenry: approval‑mozilla‑aurora+
Details | Review
MozReview Request: Bug 823483 patch 2 - Add frame state bit to indicate frame classes that do replaced-element-like sizing. r?dholbert (58 bytes, text/x-review-board-request)
2016-01-29 19:16 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
dholbert: review+
lhenry: approval‑mozilla‑aurora+
Details | Review
MozReview Request: Bug 823483 patch 3 - Limit effect of percentage width and max-width on intrinsic size to elements with replaced element sizing. r?dholbert (58 bytes, text/x-review-board-request)
2016-01-29 19:16 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
dholbert: review+
lhenry: approval‑mozilla‑aurora+
Details | Review
MozReview Request: Bug 823483 patch 4 - Make a percentage max-width override a fixed width for replaced element intrinsic size computation. r?dholbert (58 bytes, text/x-review-board-request)
2016-01-29 19:16 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
dholbert: review+
lhenry: approval‑mozilla‑aurora+
Details | Review
MozReview Request: Bug 823483 patch 5 - Make (again) percentage width on text inputs make intrinsic minimum width be 0. r?dholbert (58 bytes, text/x-review-board-request)
2016-01-29 19:16 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
dholbert: review+
lhenry: approval‑mozilla‑aurora+
Details | Review
MozReview Request: Bug 823483 patch 6 - Tests r?dholbert (58 bytes, text/x-review-board-request)
2016-01-29 19:16 PST, David Baron :dbaron: ⌚️UTC-7 (busy September 14-25)
dholbert: review+
lhenry: approval‑mozilla‑aurora+
Details | Review

Description Tim Hunt 2012-12-20 04:59:48 PST
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121128204232

Steps to reproduce:

I am experiencing this problem with a complex form, but there is a nice explanation at http://stackoverflow.com/questions/10672586/how-to-make-select-elements-shrink-to-max-width-percent-style-within-fieldset, see the "Clarification:" bit, and there is a minimal test-case at http://jsfiddle.net/e2H82/.

Applying min-width: X% to a select drop-down works fine, unless the select is nested somewhere inside a fieldset.

That is, with HTML like
<fieldset style="background:blue;">
 <select name=countries style="max-width:90%;">
  <option value=gs>South Georgia and the South Sandwich Islands</option>
 </select>
</fieldset>
 <select name=countries style="max-width:90%;">
  <option value=gs>South Georgia and the South Sandwich Islands</option>
 </select>


Actual results:

Make the browser window very narrow. The second select starts to shrink when the max-width:90%; kicks in. The first select never gets any narrower.


Expected results:

Both selects should size in the same way.

I think the same bug also affects textareas, but I have not made the same effort to cleanly reproduce that.

As an additional verification, if I use firebug to edit the fieldset tag in my original complex form to a div, then the max-width style rules on the select and textareas start working.

I don't think I have created a duplicate here. Searching, it took ma a long time to find anything (so, if I have created a duplicate, at least it will help people searching). The only vaguely close things I found were:

https://bugzilla.mozilla.org/show_bug.cgi?id=261037
https://bugzilla.mozilla.org/show_bug.cgi?id=504622

Those might possibly be duplicated, but determining that requires more knowledge of the Gecko rendering engine than I posess.
Comment 1 Boris Zbarsky [:bz] (TPAC) 2012-12-20 10:31:02 PST
The problem is that percentage max-width on the <select> is a percentage of the width of the <fieldset>.

So the real question is what the width of that <fieldset> is.

And the problem you're running into is that a fieldset will never become narrower than the claimed min-intrinsic width of its kids (bug 504622, as you noted).

That said, it's a bit odd that setting max-width to a percentage doesn't affect the min-intrinsic width the same way that setting width to a percentage does...
Comment 2 Boris Zbarsky [:bz] (TPAC) 2012-12-20 10:50:35 PST
David, should we try to have percentage max-width effectively set intrinsic min-width to 0 like percentage width does?  At least as long as the min-width is not set...
Comment 3 Boris Zbarsky [:bz] (TPAC) 2012-12-20 11:09:38 PST
Hmm.  Maybe percentage width doesn't affect intrinsic min-width either in some cases?  Certainly this testcase:

  <!DOCTYPE html>
  <body style="width: 0">
    <div style="float: left; background: green">
      <span style="display: inline-block; width: 100%;
                   background: rgba(255, 0, 0, 0.5)">
        SomeLongText
      </span>
    </div>
  </body>

does not show it doing so.  But it does seem to for <select>; if I replace the <span> there with a <select> the float width collapses to 0...  I have no idea what's going on there.
Comment 4 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2012-12-21 17:39:44 PST
(In reply to Boris Zbarsky (:bz) from comment #2)
> David, should we try to have percentage max-width effectively set intrinsic
> min-width to 0 like percentage width does?  At least as long as the
> min-width is not set...

Seems reasonable.  Presumably you'd split the middle part of this && in nsLayoutUtils::IntrinsicForContainer into an ||:

  else if (aType == MIN_WIDTH &&
           // The only cases of coord-percent-calc() units that
           // GetAbsoluteCoord didn't handle are percent and calc()s
           // containing percent.
           styleWidth.IsCoordPercentCalcUnit() &&
           aFrame->IsFrameOfType(nsIFrame::eReplaced)) {
    // A percentage width on replaced elements means they can shrink to 0.
    result = 0; // let |min| handle padding/border/margin
  }


I think the min-width overriding that would be taken care of by the code about 15 lines below.
Comment 5 Boris Zbarsky [:bz] (TPAC) 2013-01-17 20:48:59 PST
Hmm.  If I do it that way, then the percentage max-width will only set min-intrinsic to 0 if there is no explicit width set, right?  What behavior do we want in cases like:

  <img style="width: 100px; max-width: 100%">

?  For what it's worth, it looks like WebKit has the max-width win in that case, while for Opera the behavior is different for <select> and <img> at first glance (for <select> the max-width wins, but for <img> the width wins)...
Comment 6 Boris Zbarsky [:bz] (TPAC) 2013-01-17 20:49:34 PST
In either case, fixing this bug really should fix bug 441046 and bug 434230 if I'm not screwing up my testing.
Comment 7 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2013-01-21 09:28:14 PST
(In reply to Boris Zbarsky (:bz) from comment #5)
> Hmm.  If I do it that way, then the percentage max-width will only set
> min-intrinsic to 0 if there is no explicit width set, right?

Ah, true.

> What behavior
> do we want in cases like:
> 
>   <img style="width: 100px; max-width: 100%">
> 
> ?

Hmmm.  I'm not entirely sure.

And, actually, I'm now reconsidering my answer in comment 4, since when 'width' is set we've explicitly thrown away the intrinsic width, whereas when 'max-width' is set, we haven't explicitly thrown it away unless there's a value of 'width', in which case we might want to honor that.

I suppose I should think about this more...
Comment 8 Francesco 2015-03-23 15:08:45 PDT
Created attachment 8582012 [details]
workaround.htm

For me (Win7, FF 36) wrapping the select within an element styled like this
{
 display: table;
 table-layout: fixed;
 width: 100%;
}
fixes the issue (compare with the test case at http://jsfiddle.net/e2H82/).

See also bug 504622.
Comment 9 Karl Dubost :karlcow 2015-07-09 00:33:43 PDT
Webcompat issue https://webcompat.com/issues/1231
Comment 10 Karl Dubost :karlcow 2015-07-14 17:01:15 PDT
Webcompat issue https://webcompat.com/issues/1390
Comment 11 Karl Dubost :karlcow 2015-07-26 16:40:58 PDT
Probably related to this bug.
https://webcompat.com/issues/1461
Comment 12 Karl Dubost :karlcow 2015-07-26 18:11:01 PDT
Yet another one. 
https://webcompat.com/issues/1467
Comment 13 Hallvord R. M. Steen [:hallvors] 2015-07-31 08:26:02 PDT
and https://webcompat.com/issues/1477 , I presume
Comment 14 Karl Dubost :karlcow 2015-09-02 03:37:59 PDT
and https://webcompat.com/issues/1640 another one
Comment 15 Karl Dubost :karlcow 2015-09-07 23:15:08 PDT
and https://webcompat.com/issues/1343 Our monthly intrinsic width webcompat issue.
Comment 16 Karl Dubost :karlcow 2015-09-07 23:23:14 PDT
Is it related to this part of the code?
https://dxr.mozilla.org/mozilla-central/source/layout/base/nsLayoutUtils.cpp#4275
Comment 17 Daniel Holbert [:dholbert] 2015-09-16 13:04:29 PDT
*** Bug 1139931 has been marked as a duplicate of this bug. ***
Comment 18 Karl Dubost :karlcow 2015-09-22 19:07:26 PDT
Adding another one. https://webcompat.com/issues/376
Comment 19 Karl Dubost :karlcow 2015-10-14 16:17:52 PDT
Adding another one. https://webcompat.com/issues/1728
Comment 20 Hallvord R. M. Steen [:hallvors] 2015-10-16 07:16:22 PDT
Right now I simply assume reports about elements that have max-width:n% set and end up wider than expected are instances of this bug. I volunteer to do some retesting of the dupes I've closed when there is a fix to make sure there isn't some other bug with the same symptom ;)
Comment 21 Hallvord R. M. Steen [:hallvors] 2015-10-19 07:15:59 PDT
https://webcompat.com/issues/1533 has *something* to do with max-height/max-width, but I'm not quite sure if it's about this bug or something else..
Comment 22 Daniel Holbert [:dholbert] 2015-10-19 13:33:54 PDT
*** Bug 1215103 has been marked as a duplicate of this bug. ***
Comment 23 Karl Dubost :karlcow 2015-10-19 22:23:27 PDT
Adding https://webcompat.com/issues/862
Comment 24 Karl Dubost :karlcow 2015-10-25 19:23:20 PDT
Another issue with max-width
https://webcompat.com/issues/1837
Comment 25 Karl Dubost :karlcow 2015-10-25 19:24:30 PDT
And https://webcompat.com/issues/1838
Comment 26 Karl Dubost :karlcow 2015-11-16 18:33:24 PST
Adding another one. 
https://webcompat.com/issues/1936
Comment 27 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2015-11-18 12:58:20 PST
Need to respond to this thread:
https://lists.w3.org/Archives/Public/www-style/2015Nov/0225.html
Comment 28 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2015-11-20 15:11:58 PST
Created attachment 8690286 [details]
some tests with <img>
Comment 29 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2015-11-20 15:16:18 PST
Created attachment 8690287 [details]
some tests with empty <input type="button">
Comment 30 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2015-11-20 15:19:49 PST
Created attachment 8690291 [details]
some tests with <input type="button">
Comment 31 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2015-11-20 16:11:35 PST
So Gecko and IE11 actually match pretty closely right now; the only differences I see are on the width:% (not min-width or max-width %) cases for the non-empty button, and on the intrinsic width of the empty button.

Chromium is substantially different for anything involving % max-width on image.  But Chromium and IE appear to match exactly on all the <input type="button"> cases.

I'm curious whether WebKit is any different from Chromium, and whether Edge is any different from IE.  (Ubuntu's Webkit-GTK matches Chromium on intrinsic widths but not final widths for the images, and has differences on the empty button, maybe related to margin.)  I can't test Edge.



Do all the testcases duped against this involve max-width on *images*, or are there other sorts of elements involved?
Comment 32 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2015-11-20 16:28:24 PST
Created attachment 8690324 [details]
First testcase rendered in Edge
Comment 33 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2015-11-20 16:28:58 PST
Created attachment 8690325 [details]
Second testcase rendered in Edge
Comment 34 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2015-11-20 16:29:27 PST
Created attachment 8690326 [details]
Third testcase rendered in Edge
Comment 35 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2015-11-20 16:38:19 PST
So Edge changed (relative to IE11) on the image testcase so that Edge and Chromium now match on all 3 testcases.
Comment 36 percyley 2015-12-04 04:47:09 PST
Created attachment 8695836 [details]
img-testcase.html

Almost most of the layout will be affected, I hope to be able to repair as soon as possible.
Comment 37 percyley 2015-12-04 04:48:00 PST
Created attachment 8695837 [details]
img-testcase-Firefox.png
Comment 38 percyley 2015-12-04 04:48:30 PST
Created attachment 8695838 [details]
img-testcase-Chrome.png
Comment 39 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2015-12-06 13:17:03 PST
I looked into the Chromium behavior a bit more closely, and now I have patches for this, but I still need to write some tests.
Comment 41 percyley 2015-12-06 21:44:05 PST
(In reply to David Baron [:dbaron] ⌚UTC-5 from comment #39)
> I looked into the Chromium behavior a bit more closely, and now I have
> patches for this, but I still need to write some tests.

Thank you for your work.
Comment 42 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2015-12-08 18:55:04 PST
(In reply to David Baron [:dbaron] ⌚UTC-5 from comment #27)
> Need to respond to this thread:
> https://lists.w3.org/Archives/Public/www-style/2015Nov/0225.html

Responded at https://lists.w3.org/Archives/Public/www-style/2015Dec/0117.html
Comment 43 Karl Dubost :karlcow 2015-12-16 21:14:23 PST
Another one.
https://webcompat.com/issues/2028

img with max-width: 100% inside a table. So probably related to Bug 434230
Comment 44 Karl Dubost :karlcow 2015-12-21 00:16:37 PST
Our weekly webcompat issue.
https://webcompat.com/issues/1740
Comment 45 Karl Dubost :karlcow 2016-01-06 18:21:36 PST
Another one.
https://webcompat.com/issues/1805
Comment 46 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-27 17:55:18 PST
Sorry for the delay; picked this up again after holidays, standards meetings, and vacation.

Finished up with one more patch needed to avoid a compatibility regression (for text inputs, which have their own special behavior), and now a try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=44bb0fcc6332
Comment 47 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-29 14:14:37 PST
https://treeherder.mozilla.org/#/jobs?repo=try&revision=87d54e79fbeb
Comment 48 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-29 19:16:43 PST
Created attachment 8713919 [details]
MozReview Request: Bug 823483 patch 1 - Check for percentage max-width in addition to percentage width when deciding to ignore intrinsic min-width of replaced elements.  r?dholbert

This (modulo changes in later patches) matches the behavior of Chromium
and Edge.  It increases the set of elements to which this quirky
behavior applies.

Review commit: https://reviewboard.mozilla.org/r/32887/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32887/
Comment 49 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-29 19:16:45 PST
Created attachment 8713920 [details]
MozReview Request: Bug 823483 patch 2 - Add frame state bit to indicate frame classes that do replaced-element-like sizing.  r?dholbert

This is needed for patch 3.

Review commit: https://reviewboard.mozilla.org/r/32889/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32889/
Comment 50 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-29 19:16:48 PST
Created attachment 8713921 [details]
MozReview Request: Bug 823483 patch 3 - Limit effect of percentage width and max-width on intrinsic size to elements with replaced element sizing.  r?dholbert

This reduces the set of elements to which this quirky behavior applies.

This matches the behavior of Chromium and Edge.

Review commit: https://reviewboard.mozilla.org/r/32891/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32891/
Comment 51 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-29 19:16:50 PST
Created attachment 8713922 [details]
MozReview Request: Bug 823483 patch 4 - Make a percentage max-width override a fixed width for replaced element intrinsic size computation.  r?dholbert

This just reorders the if-else chain to change which conditions are
tested first.  Prior to patch 1, the order didn't matter, but with patch
1, the order does matter, and the order that we happened to have was the
opposite of the one that matches Chromium and Edge.

Review commit: https://reviewboard.mozilla.org/r/32893/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32893/
Comment 52 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-29 19:16:52 PST
Created attachment 8713923 [details]
MozReview Request: Bug 823483 patch 5 - Make (again) percentage width on text inputs make intrinsic minimum width be 0.  r?dholbert

This restores the quirky behavior for text inputs, which was removed by
patch 2, but only halfway (for width but not max-width), which matches
Chromium and Edge.

Review commit: https://reviewboard.mozilla.org/r/32895/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32895/
Comment 53 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-29 19:16:54 PST
Created attachment 8713924 [details]
MozReview Request: Bug 823483 patch 6 - Tests  r?dholbert

Our behavior on these tests is reasonably close to matching Chromium
thanks to the combination of patches 1, 3, and 4, and 5.

FIXME: Make min-intrinsic-with-percents-across-elements less platform-specific.

Review commit: https://reviewboard.mozilla.org/r/32897/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32897/
Comment 54 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-01-29 19:18:28 PST
(In reply to David Baron [:dbaron] ⌚️UTC+11 (busy, returning 8 February) from comment #53)
> FIXME: Make min-intrinsic-with-percents-across-elements less
> platform-specific.

Er, I did that (with the visibility:hidden stuff), but forgot to remove the FIXME.
Comment 55 Daniel Holbert [:dholbert] 2016-02-03 13:05:58 PST
Comment on attachment 8713919 [details]
MozReview Request: Bug 823483 patch 1 - Check for percentage max-width in addition to percentage width when deciding to ignore intrinsic min-width of replaced elements.  r?dholbert

https://reviewboard.mozilla.org/r/32887/#review30165
Comment 56 Daniel Holbert [:dholbert] 2016-02-03 13:10:31 PST
Comment on attachment 8713920 [details]
MozReview Request: Bug 823483 patch 2 - Add frame state bit to indicate frame classes that do replaced-element-like sizing.  r?dholbert

https://reviewboard.mozilla.org/r/32889/#review30167
Comment 57 Daniel Holbert [:dholbert] 2016-02-03 13:20:30 PST
Comment on attachment 8713921 [details]
MozReview Request: Bug 823483 patch 3 - Limit effect of percentage width and max-width on intrinsic size to elements with replaced element sizing.  r?dholbert

https://reviewboard.mozilla.org/r/32891/#review30171
Comment 58 Daniel Holbert [:dholbert] 2016-02-03 13:22:36 PST
Comment on attachment 8713922 [details]
MozReview Request: Bug 823483 patch 4 - Make a percentage max-width override a fixed width for replaced element intrinsic size computation.  r?dholbert

https://reviewboard.mozilla.org/r/32893/#review30175
Comment 59 Daniel Holbert [:dholbert] 2016-02-03 13:27:12 PST
Comment on attachment 8713923 [details]
MozReview Request: Bug 823483 patch 5 - Make (again) percentage width on text inputs make intrinsic minimum width be 0.  r?dholbert

https://reviewboard.mozilla.org/r/32895/#review30177
Comment 60 Daniel Holbert [:dholbert] 2016-02-03 13:44:48 PST
Comment on attachment 8713924 [details]
MozReview Request: Bug 823483 patch 6 - Tests  r?dholbert

https://reviewboard.mozilla.org/r/32897/#review30179

(Remember to drop the FIXME from your extended commit message before landing; you've probably already done that locally.)

::: layout/reftests/css-sizing/min-intrinsic-with-percents-across-elements-ref.html:2
(Diff revision 1)
> +<title>Tests for bug 823483</title>

Consider s/Tests/Reference/ in the reference case title, so that it's easier to tell which file is which when viewing the testcase & reference case alongside each other (by simply looking at the titlebar).

(I can't quickly tell which is which from the URL bar, because the "-ref.html" suffix is pushed off the end of the URL bar.)

::: layout/reftests/css-sizing/min-intrinsic-with-percents-across-elements-ref.html:43
(Diff revision 1)
> +  <td>canvas, unstyled</td>

Observation: from diffing the testcase & reference case, it looks like the "unstyled"/"almost-unstyled" cases use identical markup between the testcase & reference case.

So, I'm not sure those chunks are technically testing anything, since the identical markup will clearly render the same between testcase & reference case.

I guess you have them there for completeness, which makes some sense. If you can think of a way to make them more usefully tested, great; otherwise, not a big deal I suppose.

::: layout/reftests/css-sizing/min-intrinsic-with-percents-across-elements.html:122
(Diff revision 1)
> +

Nit: There's a stray blank line before the close-brace of the "for" loop in each file here.

That probably wants to be removed, though it doesn't matter much.
Comment 61 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-02-03 14:34:55 PST
(In reply to Daniel Holbert [:dholbert] from comment #60)
> So, I'm not sure those chunks are technically testing anything, since the
> identical markup will clearly render the same between testcase & reference
> case.
> 
> I guess you have them there for completeness, which makes some sense. If you
> can think of a way to make them more usefully tested, great; otherwise, not
> a big deal I suppose.

Yeah, not sure I can think of anything useful to do with them, and probably not worth spending time on it.
Comment 62 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-02-03 14:43:43 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee0ff2c8be1bf45c2431b4010ff1fe068a286400
Bug 823483 patch 1 - Check for percentage max-width in addition to percentage width when deciding to ignore intrinsic min-width of replaced elements.  r=dholbert

https://hg.mozilla.org/integration/mozilla-inbound/rev/3705a4614be0c45f3850deee4f1266ec92521fc7
Bug 823483 patch 2 - Add frame state bit to indicate frame classes that do replaced-element-like sizing.  r=dholbert

https://hg.mozilla.org/integration/mozilla-inbound/rev/d68daa674f403de8093ea36b03825bbff493fe25
Bug 823483 patch 3 - Limit effect of percentage width and max-width on intrinsic size to elements with replaced element sizing.  r=dholbert

https://hg.mozilla.org/integration/mozilla-inbound/rev/b34ebd7afb7811db1ef5ba36dc5c22153c99d5ce
Bug 823483 patch 4 - Make a percentage max-width override a fixed width for replaced element intrinsic size computation.  r=dholbert

https://hg.mozilla.org/integration/mozilla-inbound/rev/4e6a5f77ef94e26e74a19268b0c8b69bb39d3b55
Bug 823483 patch 5 - Make (again) percentage width on text inputs make intrinsic minimum width be 0.  r=dholbert

https://hg.mozilla.org/integration/mozilla-inbound/rev/623934e9db8ab60eb605fba84e03e882651e8f02
Bug 823483 patch 6 - Tests  r=dholbert
Comment 63 Karl Dubost :karlcow 2016-02-03 18:14:37 PST
David, Daniel, 
once you have done the review dance, I volunteer to go through all sites with the issues and see how we fix (and hopefully not break things).

Thanks a lot. <3
Comment 64 percyley 2016-02-03 19:44:46 PST
There are still problems in Flexbox. http://gtms02.alicdn.com/tps/i2/TB1m3hYLFXXXXccXpXXZaUa.pXX-801-475.png
Comment 65 Daniel Holbert [:dholbert] 2016-02-03 20:06:28 PST
(In reply to percyley from comment #64)
> There are still problems in Flexbox.

I'm pretty sure that's bug 1030952, not this bug.
Comment 66 percyley 2016-02-03 20:50:41 PST
It seems that bug 1227140 is also not related to this.
Comment 68 David Baron :dbaron: ⌚️UTC-7 (busy September 14-25) 2016-02-08 00:05:17 PST
Comment on attachment 8713919 [details]
MozReview Request: Bug 823483 patch 1 - Check for percentage max-width in addition to percentage width when deciding to ignore intrinsic min-width of replaced elements.  r?dholbert

Approval Request Comment for all 6 patches
[Feature/regressing bug #]: none
[User impact if declined]: bad layout on Web pages; this is a high-priority Web compatibility fix
[Describe test coverage new/current, TreeHerder]: a few reftests in the tree
[Risks and why]: I think the changes are relatively low risk since they change our behavior in a way that moves us closer to other browsers.  However, there's always some risk in changing behavior.  In this case, however, the changes seem likely to help.
[String/UUID change made/needed]: no
Comment 69 Liz Henry (:lizzard) (needinfo? me) 2016-02-09 08:22:00 PST
Comment on attachment 8713919 [details]
MozReview Request: Bug 823483 patch 1 - Check for percentage max-width in addition to percentage width when deciding to ignore intrinsic min-width of replaced elements.  r?dholbert

Improves web compatibility, please uplift to aurora.
Comment 71 Daniel Holbert [:dholbert] 2016-07-14 13:12:18 PDT
*** Bug 1285906 has been marked as a duplicate of this bug. ***

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