434230 - max-width: 100% works differently from width: 100% for images in tables

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Martijn Wargers (dead)]

Attachment: testcase

On myspace this is apparently used as a way to make the images in tables smaller so they don't make the table as a whole wider.

See testcase, the 2 images are the same.
On branch, the 2 tables look the same, but on trunk, the first table is wider, because max-width: 100% acts differently on trunk.
This regressed when the reflow branch landed.

Comment 2

[Daniel Holbert [:dholbert]]

See also these examples from Bug 438588:
   testcase 1  (attachment 324715 [details])
   reference 1 (attachment 324718 [details])

Comment 3

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

See CSS 2.1 specs for max-width relating intrinsic width and height:

http://www.w3.org/TR/CSS21/visudet.html#min-max-widths

«However, for replaced elements with an intrinsic ratio and both 'width' and 'height' specified as 'auto', the algorithm is as follows:

Select from the table the resolved height and width values for the appropriate constraint violation. Take the max-width and max-height as max(min, max) so that min ≤ max holds true. In this table w and h stand for the results of the width and height computations ignoring the 'min-width', 'min-height', 'max-width' and 'max-height' properties. Normally these are the intrinsic width and height, but they may not be in the case of replaced elements with intrinsic ratios.»

Notice indeed the behaviour with max-width is different with table-layout:fixed. This is because "resolved height and width values for the appropriate constraint violation" are different for table-layout:auto tables. So IMO this bug is a WONTFIX.

Comment 4

[Daniel Holbert [:dholbert]]

(In reply to comment #3)
This part is also relevant:
"<percentage>
    Specifies a percentage for determining the used value. The percentage is calculated with respect to the width of the generated box's containing block."

In Martijn's testcase, the width of the containing block is 50px, so the percentage should apply to that, AFAICT.  100% * 50px = 50px.

> Select from the table the resolved height and width values for the appropriate
> constraint violation.

Here, the constraint violation would be that the image's intrinsic width, 100px, is larger than its max-width, 100% = 50px.

> In this table w and h stand for...

Looking at the table, we see that our constraint violation matches the first line, "w > max-width".  The resolution listed there is to use the max-width (50px in this case) instead of the intrinsic width.

So, this seems clearly like a bug to me.

> Notice indeed the behaviour with max-width is different with
> table-layout:fixed. This is because "resolved height and width values for the
> appropriate constraint violation" are different for table-layout:auto tables.
> So IMO this bug is a WONTFIX.

I don't see how the fact that we do something different using the fixed-layout algorithm is relevant, or how it would make this a WONTFIX.

Comment 5

[Daniel Holbert [:dholbert]]

The only leeway I see for the current FF3 behavior to be correct is in this part of http://www.w3.org/TR/CSS21/visudet.html#min-max-widths :

"<percentage>
    [ ... ] If the containing block's width depends on this element's width, then the resulting layout is undefined in CSS 2.1. "

In this bug's testcase, the containing block's width *does* partially depend on this element's width, because the containing block is a table cell, which will expand to fit its contents if it needs to.  If this reasoning applies, then CSS2.1 doesn't define what we need to do here, in which case we're still compliant.

That argument doesn't feel particularly satisfying to me, though.  And even if it's valid and the correct behavior is unspecified, I'd still be a bit uncomfortable about the (a) regression aspect of this bug (vs FF2), and (b) the unintuitiveness of our current behavior. (i.e. how a percent value for "width" can clamp the size, but a percent value for "max-width" is useless here)

Comment 6

[Lucas Malor (mail: c6kfnkn2uc AT snkmail DOT c0m)]

(In reply to comment #4)
> Looking at the table, we see that our constraint violation matches the first
> line, "w > max-width".
(In reply to comment #5)
> "<percentage> [ ... ] If the containing block's width depends on this 
> element's width, then the resulting layout is undefined in CSS 2.1"
> 

Yes, you are pretty right, this is the real problem. I misunderstood the piece of specs I posted.

I think the bug can be fixed if percentual value of a property (1) for an element with an intrinsic ratio will be calculated taking related specified value of containing box, if it has one.

(1) for "a property" I mean width and height, and their max- and min- properties.

Comment 7

[Samuel Sidler (old account; do not CC)]

dholbert: Want to take a stab at fixing this one?

Comment 8

[Boris Zbarsky [:bzbarsky]]

The key difference between width and max-width here is this part in nsLayoutUtils::IntrinsicForContainer:

1959   else if (aType == MIN_WIDTH && eStyleUnit_Percent == styleWidth.GetUnit() &&
1960            aFrame->IsFrameOfType(nsIFrame::eReplaced)) {
1961     // A percentage width on replaced elements means they can shrink to 0.
1962     result = 0; // let |min| handle padding/border/margin
1963   }

From a spec point of view the issue is that we're not even hitting any of the parts of the spec referenced above in this bug, because the real issue is at the "compute the intrinsic width of this table" part of layout, after which we'll go through and apply the max-width, etc.

And the key issue there is that since table cells _do_ expand to contain their content they have to propagate the intrinsic min-width of said content out in their own intrinsic min-width.

We should probably do something sane with max-width when computing the intrinsic min-width of a replaced element, if we want behavior here to change.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Note, by the way, that the CSS constraint resolution stuff does NOT actually guarantee w <= (computed max-width), because we could have min-width > max-width.  Extra bonus point fun when one's a percentage and the other is a length!

So doing this needs a bit of careful thought.

Comment 10

[Boris Zbarsky [:bzbarsky]]

In particular, we already kinda screw up something like this:

  <table><tr><td width="50">
    <img ... style="width: 100%; min-width: 200%">
  </td></tr></table>

in the sense that the image overflows the table cell.  Then again, Safari and Opera have the same behavior (with Opera sizing the cell to wider than 50px to boot).

Comment 11

[Boris Zbarsky [:bzbarsky]]

So as far as other browser behavior:

Quirks mode:
IE7/Win: Image sizes as Gecko trunk, table cells both 100px wide
Safari 3.1.2/Mac: Same as Gecko 1.8.
Opera 9.6: Same as Gecko trunk.

Standards mode:
IE7/Win: Image sizes as Gecko 1.8, table cell with "width:100%" is 100px wide,
         the max-width one is 50px wide.
All other browsers as in quirks mode.

I don't have IE8 on hand to test what it does in its standards mode.

Given the above, I would be tempted to not actually change this without the spec being _very_ clear on the "correct" rendering.

Comment 12

[Mihail Yakimenko]

Good bug. I have:

div[display: table; width: 100%;]>div[display: table-cell;]>img[max-width: 100%;] Not woks, if i delete form first div display: table; all works with with.... but... https://www.monosnap.com/image/m66FqCEo2Uppxj8aJH7lHbhP6yK7UM

Comment 13

[Karl Dubost💡 :karlcow]

I added a test case on Bug 975632 to exhibit the issue.
https://bugzilla.mozilla.org/attachment.cgi?id=8556273

Is there anything we can do to push this forward? be test cases, codes, examples?

Comment 14

[Francesco]

Attachment: workaround.htm

For me (Win7, FF 36) wrapping the first image within an element styled like this
{
 display: table;
 table-layout: fixed;
 width: 100%;
}
fixes the issue.

We could also apply table-layout: fixed to the whole table, but I wish to emphasize the similarity with workarounds for other bugs (bug 504622, bug 823483, bug 932996) which do not involve a table.

Comment 15

[Karl Dubost💡 :karlcow]

Another report of WebCompat issues because of this
https://github.com/webcompat/web-bugs/issues/862

Comment 16

[Karl Dubost💡 :karlcow]

Another compat report. This time Stackoverflow
https://webcompat.com/issues/1342

Comment 17

[Loic]

The testcase works now, it has been fixed by bug 823483 which is in Nightly.

Comment 18

[Loic]

dbaron, could we close this bug too?

Comment 19

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

Sure