[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 #201335|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #18 from Alexandru Chiculita <achicu at adobe.com>  2013-05-13 09:46:02 PST ---
(From update of attachment 201335)
View in context: https://bugs.webkit.org/attachment.cgi?id=201335&action=review

Thanks for reworking this. I still have a couple nits below.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:503
> +            // In the offsetParent algorithm, the nearest ancestor search skips from the topmost named flow objects directly

nit: I would mention this only applies on elements inside the regions flow: "In the offsetParent algorithm defined for elements inside CSS Regions flows," . Also add a link to that section here.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:504
> +            // to the body element, so the offsetParent might be the body

nit: I think "so the offsetParent might be the body" is not necessary. Make sure the phrases end with a dot.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:510
> +            while (!curr->isRenderNamedFlowThread() && curr != offsetParent) {

Can offsetParent be null here? In that case you may need to change the order of the checks to avoid a null dereference.

> Source/WebCore/rendering/RenderObject.cpp:3019
> +        // The search reached the named flow thread, skip to the body element

nit: Missing dot. http://www.webkit.org/coding/coding-style.html#comments-sentences

> Source/WebCore/rendering/RenderObject.cpp:3020
> +        return document()->body()->renderBoxModelObject();

I think body() can return 0 in some early cases. You might as well just patch "curr" and let it go through.

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