Bug 152588 - Webkit not recalculating width when changing height of absolute-positioned div with image
Summary: Webkit not recalculating width when changing height of absolute-positioned di...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-29 12:02 PST by Mingwei Samuel
Modified: 2021-10-16 23:30 PDT (History)
5 users (show)

See Also:


Attachments
Page that demonstrates bug (2.03 KB, text/html)
2015-12-29 12:02 PST, Mingwei Samuel
no flags Details
Test reduction (411 bytes, text/html)
2015-12-31 14:02 PST, zalan
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mingwei Samuel 2015-12-29 12:02:33 PST
Created attachment 267987 [details]
Page that demonstrates bug

StackOverflow issue, has an example you can run:
http://stackoverflow.com/questions/34506314/webkit-bug-rendering-image-in-centered-absolute-div

Basically I have a relatively position div, with a absolutely positioned div child.  Inside the absolutely positioned div is a (square) image:
<div relative>
 <div absolute>
  <img />
 </div>
</div>

Both absolute and img have height: 100%, so when relative changes height, so should both children. The image should then recalculate it's width to preserve it's aspect ratio, which in turn will change its parent (absolute) to change width. However, when relative's height is changed (via javascript or the debugger) the widths are not properly updated. If relative's height is decreased, no widths are recalculated and the image becomes squished vertically. if relative's height is increased, the image's width is increased to maintain the aspect ratio, but its parent (absolute) does not increase width, causing the image to overflow and be displaced to the right (in the example, it should stay centered).

Attached is an example page that shows this bug (also included in the stackoverflow).
Comment 1 zalan 2015-12-31 14:02:05 PST
Created attachment 268055 [details]
Test reduction
Comment 2 zalan 2015-12-31 18:05:21 PST
(In reply to comment #0)
> Created attachment 267987 [details]
> Page that demonstrates bug
> 
> StackOverflow issue, has an example you can run:
> http://stackoverflow.com/questions/34506314/webkit-bug-rendering-image-in-
> centered-absolute-div
> 
> Basically I have a relatively position div, with a absolutely positioned div
> child.  Inside the absolutely positioned div is a (square) image:
> <div relative>
>  <div absolute>
>   <img />
>  </div>
> </div>
> 
> Both absolute and img have height: 100%, so when relative changes height, so
> should both children. The image should then recalculate it's width to
> preserve it's aspect ratio, which in turn will change its parent (absolute)
> to change width. However, when relative's height is changed (via javascript
> or the debugger) the widths are not properly updated. If relative's height
> is decreased, no widths are recalculated and the image becomes squished
> vertically. if relative's height is increased, the image's width is
> increased to maintain the aspect ratio, but its parent (absolute) does not
> increase width, causing the image to overflow and be displaced to the right
> (in the example, it should stay centered).
> 
> Attached is an example page that shows this bug (also included in the
> stackoverflow).
You could add "-webkit-aspect-ratio: from-intrinsic;" to your img element to preserve ratio.
Comment 3 zalan 2015-12-31 18:21:27 PST
It looks like we don't dirty the preferred min width on the image renderer, so we always end up with the original width.
Comment 4 zalan 2016-01-01 21:26:45 PST
diff --git a/Source/WebCore/rendering/RenderBlockFlow.cpp b/Source/WebCore/rendering/RenderBlockFlow.cpp
index 286649c..96ed8c2 100644
--- a/Source/WebCore/rendering/RenderBlockFlow.cpp
+++ b/Source/WebCore/rendering/RenderBlockFlow.cpp
@@ -479,6 +479,7 @@ void RenderBlockFlow::layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalH
     LayoutUnit maxFloatLogicalBottom = 0;
     if (!firstChild() && !isAnonymousBlock())
         setChildrenInline(true);
+    dirtyForLayoutFromPercentageHeightDescendants();
     if (childrenInline())
         layoutInlineChildren(relayoutChildren, repaintLogicalTop, repaintLogicalBottom);
     else
@@ -584,8 +585,6 @@ void RenderBlockFlow::layoutBlock(bool relayoutChildren, LayoutUnit pageLogicalH
 
 void RenderBlockFlow::layoutBlockChildren(bool relayoutChildren, LayoutUnit& maxFloatLogicalBottom)
 {
-    dirtyForLayoutFromPercentageHeightDescendants();
-
     LayoutUnit beforeEdge = borderAndPaddingBefore();
     LayoutUnit afterEdge = borderAndPaddingAfter() + scrollbarLogicalHeight();
 
^^It fixes the issue of not dirtying the replaced renderer's preferred width when the (non-direct)ancestor height changes.
 
The failing test case has 
(A)RenderBlock with fixed height.
  (B)RenderBlock with percentage height
    (C)RenderImage with percentage height

When (A)'s height is changed, we recalculate (C)'s height but we keep the original width (so we might lose the original aspect ratio)

In this case, gPercentHeightDescendantsMap has the (B)->(C) pair -since (A) has non-auto, non-percentage height. By changing (A)'s height, all the renderers (A)(B) and (C) are correctly marked dirty. However the preferred width bit does not get dirty (C) because the RenderBlock only calls dirtyForLayoutFromPercentageHeightDescendants() on its block children only -while here, (B) only has an inline child (C) 
Dave, do you know why it was originally added only to the block children path?
Comment 5 zalan 2016-01-11 16:53:13 PST
(B)lock/(I)nline/I(N)line-block, (R)elative/A(B)solute/Fi(X)ed/Stick(Y) positioned, (O)verflow clipping, (A)nonymous, (G)enerated, (F)loating, has(L)ayer, (C)omposited, (D)irty layout, Dirty (S)tyle
B--AG-L- --* RenderView  (0.00, 0.00) (1323.00, 455.00) renderer->(0x11596c580)
B-----L- --    HTML RenderBlock  (0.00, 0.00) (1323.00, 455.00) renderer->(0x1159ee678) node->(0x1159f1680)
B------- --      BODY RenderBody  (8.00, 8.00) (1307.00, 439.00) renderer->(0x1159ee730) node->(0x1159f1750)
B------- --        DIV RenderBlock  (0.00, 0.00) (1307.00, 100.00) renderer->(0x1159ee7e8) node->(0x1159f17b8)
B------- --          DIV RenderBlock  (0.00, 0.00) (1307.00, 100.00) renderer->(0x1159ee8a0) node->(0x1159f1888)
-------- --            RootInlineBox  (0.00, 86.00) (200.00, 18.00) (0x11a7dc888) renderer->(0x1159ee8a0)
-------- --              InlineBox  (0.00, 0.00) (200.00, 100.00) (0x11a7e62d0) renderer->(0x1159da420)
I------- --            IMG RenderImage  (0.00, 0.00) (200.00, 100.00) renderer->(0x1159da420) node->(0x11595e160)
Comment 6 Mingwei Samuel 2016-07-06 12:20:24 PDT
I noticed that in AppleWebKit/537.36 (I do not know if that is the same vendor as this site) the image width now correctly updates to stay proportional, however the parent absolute still does not update it's width, causing the image to be left-ish-aligned instead of center-aligned
Comment 7 Rob Buis 2021-10-13 00:21:17 PDT
I am not sure this can be reproduced these days?
Comment 8 Mingwei Samuel 2021-10-16 23:30:32 PDT
Seems that this issue is fixed. I can no longer produce any unexpected behavior.