Bug 58453 – Absolute positioned elements in a relative positioned CSS3 Flexbox fail to display properly

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Matt Cooper]

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.

[Loic Dachary]

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]

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]

Created attachment 142841 [details]
absolute positioned elements in flexbox container fail to stay abs. positioned.

[Eric]

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]

Created attachment 143651 [details]
more reduced test-case

[Ojan Vafai]

Created attachment 144189 [details]
Patch

[Tony Chang]

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 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]

Created attachment 144597 [details]
Patch

[Ojan Vafai]

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]

Committed r118818: <http://trac.webkit.org/changeset/118818>

[Andy Estes]

<rdar://problem/9278885>