[Webkit-unassigned] [Bug 113276] [CSSRegions] Implement offsetParent for elements inside named flow

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=113276


Alexandru Chiculita <achicu at adobe.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #200893|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #11 from Alexandru Chiculita <achicu at adobe.com>  2013-05-07 08:34:42 PST ---
(From update of attachment 200893)
View in context: https://bugs.webkit.org/attachment.cgi?id=200893&action=review

Looks good! I have a few small nits below.

> LayoutTests/ChangeLog:8
> +        Reviewed by NOBODY (OOPS!).

Nit: this line usually comes after the bug link.

> LayoutTests/fast/regions/offsetLeft-offsetTop-in-flow-thread.html:4
> +		padding: 10px;

I think it's better to specify a font size here. This way you avoid differences between default styles. Also, Ahem font could reduce the differences across platforms.

> LayoutTests/fast/regions/offsetLeft-offsetTop-in-flow-thread.html:56
> +		shouldBe("table.offsetTop", "18");

Testing for an exact value in the test file would make it hard to rebate line on other platforms.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:500
> +            // In the offsetParent algorithm, the nearest ancestor search skips from the topmost named flow elements directly to the body element.

There is no element for the names flows. They are only render objects.

> Source/WebCore/rendering/RenderObject.cpp:3015
> +    // If this element is inside a named flow thread, the nearest ancestor search skips

I would avoid to use "element" when talking about render objects. It is a little confusing.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.