Bug 58453 - Absolute positioned elements in a relative positioned CSS3 Flexbox fail to display properly
: Absolute positioned elements in a relative positioned CSS3 Flexbox fail to di...
Status: RESOLVED FIXED
: WebKit
CSS
: 528+ (Nightly build)
: All Mac OS X 10.6
: P2 Major
Assigned To:
:
: InRadar
:
: 62048
  Show dependency treegraph
 
Reported: 2011-04-13 10:28 PST by
Modified: 2012-05-31 12:03 PST (History)


Attachments
Test case for absolute-positioned element inside of a flexbox (25.25 KB, text/html)
2011-04-13 10:28 PST, 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 PST, Eric
no flags Details
more reduced test-case (855 bytes, text/html)
2012-05-23 14:24 PST, Ojan Vafai
no flags Details
Patch (34.84 KB, patch)
2012-05-25 21:03 PST, Ojan Vafai
no flags Review Patch | Details | Formatted Diff | Diff
Patch (35.60 KB, patch)
2012-05-29 12:16 PST, Ojan Vafai
tony: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-04-13 10:28:45 PST
Created an attachment (id=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.
------- Comment #1 From 2011-09-28 03:48:16 PST -------
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.
------- Comment #2 From 2011-09-30 08:38:55 PST -------
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)
------- Comment #3 From 2012-05-18 19:35:01 PST -------
Created an attachment (id=142841) [details]
absolute positioned elements in flexbox container fail to stay abs. positioned.
------- Comment #4 From 2012-05-18 19:35:58 PST -------
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.
------- Comment #5 From 2012-05-23 14:24:23 PST -------
Created an attachment (id=143651) [details]
more reduced test-case
------- Comment #6 From 2012-05-25 21:03:27 PST -------
Created an attachment (id=144189) [details]
Patch
------- Comment #7 From 2012-05-29 10:09:52 PST -------
(From update of attachment 144189 [details])
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.
------- Comment #8 From 2012-05-29 11:13:05 PST -------
(From update of attachment 144189 [details])
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.
------- Comment #9 From 2012-05-29 12:16:18 PST -------
Created an attachment (id=144597) [details]
Patch
------- Comment #10 From 2012-05-29 12:17:29 PST -------
(From update of attachment 144189 [details])
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.
------- Comment #11 From 2012-05-29 12:28:11 PST -------
Committed r118818: <http://trac.webkit.org/changeset/118818>
------- Comment #12 From 2012-05-31 12:03:13 PST -------
<rdar://problem/9278885>