Bug 157455 – Absolute positioned element is not placed properly when parent becomes the containing block.

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[zalan]

Created attachment 278345 [details]
Test case

When the parent becomes the containing block by setting transform on it. (-by using timeout)
-see test case

[zalan]

rdar://problem/26212568

[zalan]

Created attachment 278576 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 278576 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=278576&action=review

> Source/WebCore/ChangeLog:10
> +        When a container becomes a containing block, we need to check if there's any positioned box in its subtree
> +        in order to "re-parent" them. It basically means that we remove them from RenderBlock::positionedDescendants map

"...if there is ... to reparent it", or "if there are...to reparent them". Don't mix singular and plural!

> Source/WebCore/rendering/RenderBlock.cpp:243
> +    if (oldStyle.position() == newStyle.position() && hadTransform == willHaveTransform)

This is OK but I would prefer we add RenderStyle::containingBlockForPositioned(), or positionedOrTransformed() or something. I bet we need the same logic elsewhere.

> Source/WebCore/rendering/RenderBlock.cpp:260
> +            && (containingBlock->style().position() == StaticPosition || (containingBlock->isInline() && !containingBlock->isReplaced()))) {

Would be nice to wrap this conditional in a function so I know what you're checking. Why inline and not replaced! I think this pattern exists in multiple places.

Could be done separately.

[zalan]

(In reply to comment #3)
> Comment on attachment 278576 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=278576&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        When a container becomes a containing block, we need to check if there's any positioned box in its subtree
> > +        in order to "re-parent" them. It basically means that we remove them from RenderBlock::positionedDescendants map
> 
> "...if there is ... to reparent it", or "if there are...to reparent them".
> Don't mix singular and plural!
> 
> > Source/WebCore/rendering/RenderBlock.cpp:243
> > +    if (oldStyle.position() == newStyle.position() && hadTransform == willHaveTransform)
> 
> This is OK but I would prefer we add
> RenderStyle::containingBlockForPositioned(), or positionedOrTransformed() or
> something. I bet we need the same logic elsewhere.
There are a couple of places where we use similar logic (requiresLayer and canContainAbsolutelyPositionedObjects) but it seems like we explicitly write out all the requires properties there.
 
> 
> > Source/WebCore/rendering/RenderBlock.cpp:260
> > +            && (containingBlock->style().position() == StaticPosition || (containingBlock->isInline() && !containingBlock->isReplaced()))) {
> 
> Would be nice to wrap this conditional in a function so I know what you're
> checking. Why inline and not replaced! I think this pattern exists in
> multiple places.
> 
> Could be done separately.
Ok. I'll do it in a follow-up patch.

[zalan]

Committed r200736: <http://trac.webkit.org/changeset/200736>