1491936 - parent width with scaled <img> take former intrinsic size of img and not resized one

Status: NEW

(Core :: Layout: Images, Video, and HTML Frames, defect, P3)

Description

[Notyrgd]

Attachment: example.html

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

Steps to reproduce:

https://codepen.io/Notyrgd/pen/RYeRoE

Let's say #widthwrapper's height is H
#imgwrapper has no intrinsic size. But its aspect ratio is 1/1. With height=100%=H it will have width = H
#widthwrapper should resize to fit the new width of #imgwrapper. But it doesn't...


Actual results:

#widthwrapper width=imageIntrinsicSize = 0 
img is resisez to fit the height = H, its width is also updated to H but the parent div #widthwrapper is still (w*h = 0*H)


Expected results:

#widthwrapper should be around #imgwrapper with (w*h = H*H)

Comment 1

[Notyrgd]

UP. this bug is critical and forces me to add stupid hacks or to refuse access to my website to user with firefox.
Rapid fix is needed. Any news welcome

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Teach GetPercentBSize about OOF frames.

It's likely that there's a better way to do this, but I just want to confirm
that this part of the code is the right one to tweak :)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9013298 [details]
WIP: Teach GetPercentBSize about OOF frames.

So the issue here is that we fail to find a specific bSize to resolve the image percentage against.

This is because we bailout from GetPercentBSize because the height of the fixed item is auto (not absolute or a percent), even though the fixed pos is sized definitely because the two bsize sides are non-auto.

David, this probably needs a bunch more work to deal with abspos as well, fixing the various issues and TODOs this patch has, ensuring this deals with the right writing modes, and probably some place to put the logic for computing this in a shared place, but does this look like a reasonable start?

Comment 4

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

Comment on attachment 9013298 [details]
WIP: Teach GetPercentBSize about OOF frames.

So I guess I'd be interested in understanding what it is you're trying to do, and how thoroughly you've checked that it's the same as what other browsers do.

It looks like what you're doing is treating 'auto' height on fixed pos as the height of the containing block.  That feels a little weird to me given that it's not really equivalent to that, even in amount of space available, given that the offsets reduce the amount of space available (especially with 'top:auto' positioning the top at a random point in the middle).

But it sounds like you want to do more than that, and it's not clear to me what the more is, or how interoperable the behavior that it's emulating is.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-7 from comment #4)
> Comment on attachment 9013298 [details]
> WIP: Teach GetPercentBSize about OOF frames.
> 
> So I guess I'd be interested in understanding what it is you're trying to
> do

So, what I'm trying to do is to resolve the percentage correctly, since auto-size abspos / fixed pos things with non-auto offsets fill their containing block. Yesterday I didn't have the time to look it up but that happens here:

https://searchfox.org/mozilla-central/rev/a11c128b90ea85d7bb68f00c5de52c0794e0b215/layout/generic/ReflowInput.cpp#1913

> [..] and how thoroughly you've checked that it's the same as what other
> browsers do.

Not much, but other browsers do get the reported case right, and we're interoperable for normal sizing, so I assume they'd get right the same cases. I'll test before fixing up the patch for review.

> It looks like what you're doing is treating 'auto' height on fixed pos as
> the height of the containing block.  That feels a little weird to me given
> that it's not really equivalent to that, even in amount of space available,
> given that the offsets reduce the amount of space available (especially with
> 'top:auto' positioning the top at a random point in the middle).

Not really, note that I do substract the offsets:

  aResult = max(0, bSize - bStart - bEnd)

But that's not enough per the line above. I also need to subtract margin and border-padding.