[webkit-reviews] review denied: [Bug 65477] Nested fixed position element not staying with parent : [Attachment 107982] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Robert Hogan <robert at webkit.org>'s
request for review:
Bug 65477: Nested fixed position element not staying with parent
https://bugs.webkit.org/show_bug.cgi?id=65477

Attachment 107982: Patch
https://bugs.webkit.org/attachment.cgi?id=107982&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=107982&action=review


r-

> Source/WebCore/rendering/RenderBlock.cpp:2247
> +	   if  (r->parent()->style()->position() != AbsolutePosition)
> +	       continue;

This needs to be a full-blown ancestor check all the way up the tree I think? I
don't think you can get away with just a parent check. Try making a test case
with a <span> in between the absolute positioned parent and the fixed
positioned child and you should see what I mean.

> Source/WebCore/rendering/RenderBlock.cpp:2286
> +	   if (relayoutChildren ||
((r->style()->hasStaticBlockPosition(isHorizontalWritingMode()) ||
r->style()->hasStaticInlinePosition(isHorizontalWritingMode())) 
> +	       && r->parent() != this))

I still think this check is all wrong and worry that just adding this in
unqualified is too broad. See comment #6 in the bug.

Fixing this only in simplified layout will also result in the bug not being
fixed if you have to do a full layout. Should come up with test cases for that
(dirty more to make simplified layout not trigger).