If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

intrinsic width calculations in abspos element with implicitly definite height doesn't take percent-height intrinsic-ratio descendants into account

NEW
Unassigned

Status

()

Core
Layout
P3
normal
5 months ago
2 months ago

People

(Reporter: percyley, Unassigned)

Tracking

55 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

5 months ago
Created attachment 8865427 [details]
expected.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_4) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.96 Safari/537.36

Steps to reproduce:

1. Open http://jsbin.com/jemupowobo/edit?output
2. Checked 'Resize me' button


Actual results:

The img is not fully displayed.


Expected results:

See: expected.png

At present, only use this hack fixed `height: calc(100% - 50px - 500px)`

Chrome has been fixed in: https://codereview.chromium.org/2828453002/

But Chrome has two other similar problems:

https://bugs.chromium.org/p/chromium/issues/detail?id=719441
https://bugs.chromium.org/p/chromium/issues/detail?id=719452
jsbin seems to be down now, so I can't view the testcase.
Summary: inline-block dose not update its width after parent resize causes child image resize → inline-block does not update its width after parent resize causes child image resize
(Reporter)

Comment 2

5 months ago
(In reply to David Baron :dbaron: ⌚️UTC-7 from comment #1)
> jsbin seems to be down now, so I can't view the testcase.

Now you can open it.
Comment hidden (obsolete)
Created attachment 8877768 [details]
testcase 1, original version (with a bit of extra cruft)

OK, I think I might see what you're talking about. Comparing against Chrome, with tall window heights, the .container element seems to clip the image horizontally in Firefox, when the image and container are tall (which makes the image wide but does not impact the container's width).

This seems like we might be treating the image's "height: 100%" as impossible-to-resolve when we compute intrinsic widths -- so we resolve the container's intrinsic width based on the image's intrinsic width.  But then we *do* resolve the height:100% during layout, so the image ends up taller and wider than we set aside space for.

Here's a testcase which demonstrates a version of the same problem.  Here, we've got:
 - a 600px-by-600px intrinsic-size canvas
 - ... whose height is shrunk to 90px, by virtue of it having "height: 100%" and it being inside of an abspos element with the "top" and "bottom" properties set.
 - ... and this shrinks its width as well.
 - ... but despite that, its parent ".container" has an absurdly-large intrinsic width (taken from the canvas's intrinsic width without any knowledge of the fact that its height is constrained).
Edge 15 matches our behavior, FWIW.  And Presto (pre-blink opera engine) does its own separate thing.

Behaviors on Testcase 1:
========================
* Firefox & Edge 15:         Purple canvas is small square. Red dotted container is wide rectangle.
* Opera 12.12 (presto):      Purple canvas and red dotted container are *both* wide rectangles.
* Chrome 61 and Safari 10.1: Purple canvas and red dotted container are *both* small squares.

(Here, "small" = 90px, and "wide" = 600px)
Created attachment 8877773 [details]
testcase 1

(Simplified the testcase slightly -- removed an unnecessary wrapper div and some  "float" cruft left around from an earlier version that had more nesting.)
Attachment #8877768 - Attachment is obsolete: true
Attachment #8877768 - Attachment description: testcase 1 → testcase 1, original version (with a bit of extra cruft)
Created attachment 8877784 [details]
testcase 2 (reveals brokenness around this in Edge and Blink)

Here's a slightly different reduction of the original testcase, preserving the ".box" wrapper as the abspos thing.

Firefox & Chrome each render this the same as they render "testcase 1" (and differently from each other), as described in comment 5.  Firefox renders the dotted area wide, and Chrome renders it as a square.

However: in Chrome, if you hover the purple square (making a tweak to the "bottom" property), then Chrome gets into a broken state where the dotted area is trailing the size of the purple thing that it's shrinkrapping by one layout. (So the dotted area is always too wide or too skinny.)

Edge has a different bug - it seems to decide on the width of the dotted area during the first layout (based in part on the value of "bottom" on the parent), BUT it doesn't update that width at all when I hover to dynamically update the parent's "bottom" value. It does update the size of the purple canvas and the *height* of the dotted area, though.

So, Edge and Chrome get the author's expected initial rendering (with the red dotted area shrinkwrapping the image/canvas inside of it), but their sizing is very fragile with respect to incremental changes...
Created attachment 8877787 [details]
testcase 3 (same as testcase 2 but with initial state swapped with hover state)
(In reply to Daniel Holbert [:dholbert] from comment #7)
> So, Edge and Chrome get the author's expected initial rendering (with the
> red dotted area shrinkwrapping the image/canvas inside of it), but their
> sizing is very fragile with respect to incremental changes...

I filed an issue for Edge:
  https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/12345999/

...and I commented on this existing Chrome issue that was linked in comment 0 (I think the problem I'm seeing might be related):
  https://bugs.chromium.org/p/chromium/issues/detail?id=719441
Sorry, mis-paste -- the Chrome issue I commented on was actually this one (the other from comment 0):
 https://bugs.chromium.org/p/chromium/issues/detail?id=719452
(Reporter)

Comment 11

3 months ago
@Daniel Holbert Thank you for your detailed testing. Do you have any plans to fix this problem?
I'd need to look at the spec interactions to convince myself that it's actually a bug & what the correct behavior is for variations of these testcases.  As noted above, we're not entirely on our own, on our current behavior -- we're consistent with Edge in a simplified version of the testcase ("testcase 1") -- and where we disagree, Edge isn't consistent with itself (testcase 2).

(Also: I'm mostly tasked with helping fix layout/css performance issues right now (as part of the "quantum flow" project) -- and since this isn't in that bucket, it's not at the top of my personal priority list at the moment.)

Setting needinfo=me to at least look at the specs & figure out if there's spec support for any particular behavior, though.
Flags: needinfo?(dholbert)
Also FWIW, this is not a regression -- our behavior on the attached testcases has been the same since before Firefox 4.

(Tested Nightly/Minefield 3.7a1pre 2009-12-01)
Flags: needinfo?(dholbert)
Summary: inline-block does not update its width after parent resize causes child image resize → intrinsic width calculations in abspos element with implicitly definite height doesn't take percent-height intrinsic-ratio descendants into account
So I think the closest thing to authoritative spec-text about this is in CSS2 10.3.2 "Inline, replaced elements": 

> 10.3 Calculating widths and margins
> -----------------------------------
> The values of an element's 'width' [...] propert[y] as used for layout [...]
>
> 10.3.2 Inline, replaced elements
> --------------------------------
> ... if 'width' has a computed value of 'auto',
> 'height' has some other computed value, and the
> element does have an intrinsic ratio; then the
> used value of 'width' is:
>    (used height) * (intrinsic ratio)
https://drafts.csswg.org/css2/visudet.html#inline-replaced-width

The <canvas> in my attached testcases (and the img in the reporter's JSBin testcase) fits the qualifications here -- auto width, non-auto height. This spec text makes it clear that it *is* appropriate that its used 'width' should be determined based off of its aspect ratio + used height.

The key thing here, though, is: how should the abspos element determine its width, and should this calculation benefit from the information about the image's used width (given that this derives from the abspos element's implicitly-constrained 'height' from 'top'/'bottom').  I can't find any hard text about how this is supposed to work at the moment -- the closest is CSS2 10.3.7:
> If all three of 'left', 'width', and 'right' are 'auto':
> [...] set 'left' to the static position and apply rule
>  number three below
> [...]
> 3. 'width' and 'right' are 'auto' and 'left' is not 'auto',
> then the width is shrink-to-fit 
> [...]
> CSS 2.2 does not define the exact algorithm.
https://drafts.csswg.org/css2/visudet.html#abs-non-replaced-width

I thought the intrinsic sizing spec defined shrink-to-fit, but it doesn't really seem to:
> Note: This specification does not define how to determine these sizes.
> https://drafts.csswg.org/css-sizing-3/#intrinsic-sizes

So, bottom-line -- I think the spec kind of leaves this situation undefined. BUT, I think it makes sense that we should be consistent about honoring the chain of percentage heights here -- and since we honor it for determining the final size, we should honor it for determining the container's intrinsic width as well.  So, IMO, this is a valid bug.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Daniel Holbert [:dholbert] from comment #7)
> Created attachment 8877784 [details]
> testcase 2 (reveals brokenness around this in Edge and Blink)
> [...] in Chrome, if you hover the purple square (making a tweak to the
> "bottom" property), then Chrome gets into a broken state where the dotted
> area is trailing the size of the purple thing

BTW, the Chrome folks recently landed a patch for https://bugs.chromium.org/p/chromium/issues/detail?id=719452 , and that patch fixed this ^^ issue. (Just verified on latest Chrome Canary.)  Now their dotted border stays the same size as the purple canvas that it's shrinkwrapping, even after the dynamic hover sizing-tweak.

Updated

2 months ago
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.