[webkit-reviews] review denied: [Bug 69210] CSS 2.1 failure: inline-box-002.htm fails : [Attachment 124797] 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 69210: CSS 2.1 failure: inline-box-002.htm fails
https://bugs.webkit.org/show_bug.cgi?id=69210

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

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


Really close, but not quite! ;)

> Source/WebCore/rendering/RenderInline.cpp:129
> +static bool hasRelPositionedAncestor(RenderObject* p)

I'd probably put the word Inline in this method... e.g.,
hasRelPositionedInlineAncestor

> Source/WebCore/rendering/RenderInline.cpp:145
> +    if (oldStyle->position() == RelativePosition &&
hasRelPositionedAncestor(cont))
> +	   return;

You have to do this per-block. You can't only do it for the |block| argument.
Consider this testcase:

<span style="position:relative"><span>One <div>First block</div> <span
style="position:relative">Two <div>Second block</div></span></span></span>

If you remove the position:relative on the outermost span and only check
starting at |block|'s continuation, then you'll only examin the outermost two
spans. However a later anonymous block continuation actually is split around a
deeper nesting stack, and it has a third span that has position:relative that
you won't see.

Basically you can fix this by turning this into part of the condition inside
the loop instead, so that you do it for each block.