Bug 58453

Summary: Absolute positioned elements in a relative positioned CSS3 Flexbox fail to display properly
Product: WebKit Reporter: Matt Cooper <matt11ag-webkitbugs>
Component: CSSAssignee: Ojan Vafai <ojan>
Status: RESOLVED FIXED    
Severity: Major CC: aestes, ericbidelman, eric, hyatt, jchaffraix, loic, ojan, shanestephens, tony, webkit.review.bot, wiredearp
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.6   
Bug Depends on:    
Bug Blocks: 62048    
Attachments:
Description Flags
Test case for absolute-positioned element inside of a flexbox
none
absolute positioned elements in flexbox container fail to stay abs. positioned.
none
more reduced test-case
none
Patch
none
Patch tony: review+

Matt Cooper
Reported 2011-04-13 10:28:45 PDT
Created attachment 89399 [details] Test case for absolute-positioned element inside of a flexbox Overview: If you have a CSS3 Flexbox layout the boxes display as expected (or at least mostly as expected). However, if a box has an element inside of it that uses absolute-positioning so that its boundaries are dependent on the relative box boundaries, the absolute-positioned child element fails to display properly; its bottom edge style is ignored. Steps to Reproduce: Minimized, easy-to-follow steps that will trigger the bug. Include any special setup steps. 1.) Either: a.) Open the attached use-cases.html file to see if the yellow "test" absolute-positioned DIV shows up with a 20px offset from the edges of the green flexbox. or b.) Create a CSS3 Flexbox layout where one of the boxes uses background-color:green;position:relative and then insert a DIV inside of that box and have it use a background-color:yellow style (so you can see it) and position:absolute;top:20px;right:20px;left:20px;bottom:20px. 2.) Verify that the yellow DIV is inset by 20 pixels from the top, the left, the right, and the bottom of the green DIV. Expected Results: The yellow DIV should be inset by 20 pixels from every edge of the green box (compare with Internet Explorer 10 for a visual comparison). Actual Results: WebKit does inset the yellow DIV by 20 pixels on the top, the left, and the right but fails to inset the bottom by 20 pixels. Additional Information: The attached use-cases.html file has many other use case scenarios you can test but all use cases fail in WebKit to handle absolute-positioned children properly.
Attachments
Test case for absolute-positioned element inside of a flexbox (25.25 KB, text/html)
2011-04-13 10:28 PDT, Matt Cooper 		no flags
Details
absolute positioned elements in flexbox container fail to stay abs. positioned. (1.24 KB, text/html)
2012-05-18 19:35 PDT, Eric 		no flags
Details
more reduced test-case (855 bytes, text/html)
2012-05-23 14:24 PDT, Ojan Vafai 		no flags
Details
Patch (34.84 KB, patch)
2012-05-25 21:03 PDT, Ojan Vafai 		no flags
DetailsFormatted DiffDiff
Patch (35.60 KB, patch)
2012-05-29 12:16 PDT, Ojan Vafai 		tony: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Loic Dachary
Comment 1 2011-09-28 03:48:16 PDT
I tried the attached example with libwebkit-1.0-1 version 1.4.2 on GNU/Linux and it shows that the bottom border is indeed not at 20px from the green border of the enclosing flexbox. I tried with firefox4 too and the result is the same. I tried with IE10 preview 2 and the bottom of the yellow box is indeed at 20px from the bottom of the enclosing flexbox. Do you happen to have a link to the corresponding bug report in firefox ? I wonder if and how they solve it.
Matt Cooper
Comment 2 2011-09-30 08:38:55 PDT
Here are the bugs I filed with other organizations: Bug filed with Mozilla: https://bugzilla.mozilla.org/show_bug.cgi?id=649701 Bug filed with Apple: http://bugreport.apple.com (Issue #9278885)
Eric
Comment 3 2012-05-18 19:35:01 PDT
Created attachment 142841 [details] absolute positioned elements in flexbox container fail to stay abs. positioned.
Eric
Comment 4 2012-05-18 19:35:58 PDT
I attached smaller test case that demonstrates the issue. It's using :after/:before content which is absolutely positioned as well as an abs. positioned element inside a flexbox container.
Ojan Vafai
Comment 5 2012-05-23 14:24:23 PDT
Created attachment 143651 [details] more reduced test-case
Ojan Vafai
Comment 6 2012-05-25 21:03:27 PDT
Created attachment 144189 [details] Patch
Tony Chang
Comment 7 2012-05-29 10:09:52 PDT
Comment on attachment 144189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144189&action=review You might want to find Hyatt on IRC to give this a quick look. > Source/WebCore/rendering/RenderFlexibleBox.cpp:949 > +void RenderFlexibleBox::prepareChildForPositionedLayoutHandlingRowReverse(RenderBox* child, LayoutUnit mainAxisOffset, LayoutUnit crossAxisOffset) > +{ > + LayoutUnit inlinePosition = isColumnFlow() ? crossAxisOffset : mainAxisOffset; > + if (style()->flexDirection() == FlowRowReverse) Can we just pass an enum to prepareChildForPositionedLayout to flip for row-reverse? It could default to NoFlipForRowReverse. > Source/WebCore/rendering/RenderFlexibleBox.cpp:995 > + child->layoutIfNeeded(); Do you know why this is needed? > Source/WebCore/rendering/RenderFlexibleBox.cpp:1174 > + // FIXME: Should this check be in adjustAlignmentForChild? It doesn't seem to affect any testcases. > + if (!shouldFlexAlignApplyToChild(child)) > + continue; I suspect the static inline and static block position don't matter when top/right/bottom/left are set. RenderBox::computeInlineStaticDistance bails if logicalLeft or logicalRight are set and the same happens in computeBlockStaticDistance. > Source/WebCore/rendering/RenderFlexibleBox.cpp:1276 > - child->repaintDuringLayoutIfMoved(oldRect); > + adjustAlignmentForChild(child, newOffset - originalOffset); This is much better. > LayoutTests/css3/flexbox/align-absolute-child.html:146 > + <!-- FIXME: Should positioned flex items stretch to their container? --> I don't think they should, but it would be nice to be clear about this in the spec.
Ojan Vafai
Comment 8 2012-05-29 11:13:05 PDT
Comment on attachment 144189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144189&action=review >> Source/WebCore/rendering/RenderFlexibleBox.cpp:949 >> + if (style()->flexDirection() == FlowRowReverse) > > Can we just pass an enum to prepareChildForPositionedLayout to flip for row-reverse? It could default to NoFlipForRowReverse. Could. That's what I did at first and I didn't like it as much. But I don't feel strongly. Can change it back if you prefer. >> Source/WebCore/rendering/RenderFlexibleBox.cpp:995 >> + child->layoutIfNeeded(); > > Do you know why this is needed? We don't need it for this block of code, but we need it for the later blocks that need to know the width/height of the child, e.g. in flipForWrapReverse. We could move this call lower in the code, but then we'd need to be calling layoutIfNeeded from multiple places for positioned children. >> Source/WebCore/rendering/RenderFlexibleBox.cpp:1174 >> + continue; > > I suspect the static inline and static block position don't matter when top/right/bottom/left are set. RenderBox::computeInlineStaticDistance bails if logicalLeft or logicalRight are set and the same happens in computeBlockStaticDistance. Hmmm. If that's right, then we don't need this if-statement at all. All we need is the changes to adjustAlignmentForChild. Let me try removing it.
Ojan Vafai
Comment 9 2012-05-29 12:16:18 PDT
Created attachment 144597 [details] Patch
Ojan Vafai
Comment 10 2012-05-29 12:17:29 PDT
Comment on attachment 144189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144189&action=review >>> Source/WebCore/rendering/RenderFlexibleBox.cpp:995 >>> + child->layoutIfNeeded(); >> >> Do you know why this is needed? > > We don't need it for this block of code, but we need it for the later blocks that need to know the width/height of the child, e.g. in flipForWrapReverse. We could move this call lower in the code, but then we'd need to be calling layoutIfNeeded from multiple places for positioned children. Turns out my understanding of the spec was wrong. We align the zero-width/height placeholder, not the actual positioned element, so we don't need to ever know it's width/height. >>> Source/WebCore/rendering/RenderFlexibleBox.cpp:1174 >>> + continue; >> >> I suspect the static inline and static block position don't matter when top/right/bottom/left are set. RenderBox::computeInlineStaticDistance bails if logicalLeft or logicalRight are set and the same happens in computeBlockStaticDistance. > > Hmmm. If that's right, then we don't need this if-statement at all. All we need is the changes to adjustAlignmentForChild. Let me try removing it. Yup, this check isn't needed wit the changes to adjustAlignmentForChild.
Ojan Vafai
Comment 11 2012-05-29 12:28:11 PDT
Committed r118818: <http://trac.webkit.org/changeset/118818>
Andy Estes
Comment 12 2012-05-31 12:03:13 PDT
<rdar://problem/9278885>
Note You need to log in before you can comment on or make changes to this bug.