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

Tue May 14 09:55:05 PDT 2013

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

Darin Adler <darin at apple.com> changed:

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

--- Comment #20 from Darin Adler <darin at apple.com>  2013-05-14 09:53:31 PST ---
(From update of attachment 201697)
View in context: https://bugs.webkit.org/attachment.cgi?id=201697&action=review

> Source/WebCore/rendering/RenderBoxModelObject.cpp:509
> +            // In the offsetParent algorithm defined for elements inside CSS Regions flows,
> +            // the nearest ancestor search skips from the topmost named flow object directly
> +            // to the body element.
> +            // However, the RenderNamedFlowThread is attached to the RenderView directly,
> +            // meaning that we are going to bypass the <body>'s RenderObject while iterating the parents.
> +            // As such, if the RenderNamedFlowThread is reached while iterating the parents, end this loop.
> +            // See http://dev.w3.org/csswg/css-regions/#cssomview-offset-attributes

I like the idea of this comment, but it would be much more useful if we could write a smaller one. There is so much text here that it’s hard to read. Is there a more concise way of saying the same thing? Could you follow this up with a patch to improve this comment by making it much shorter? I suggest this comment:

    // CSS region specification says that region flows should return the body element as their offsetParent.
    // Since we will bypass the body’s renderer anyway, just end the loop if we encounter a region flow (named flow thread).

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

The comment would be better if it was worded slightly differently. I would just say this:

    // CSS region specification says that region flows should return the body element as their offsetParent.

> Source/WebCore/rendering/RenderObject.cpp:3016
> +    // http://dev.w3.org/csswg/css-regions/#cssomview-offset-attributes

I don’t think the link to the spec is all that useful.

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

This comment just repeats what the code does and should be removed.

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