[Webkit-unassigned] [Bug 122963] [CSSRegions] Display anonymous regions in DRT

[bugzilla-daemon at webkit.org]

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


Alexandru Chiculita <achicu at adobe.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #214953|review?                     |review-
               Flag|                            |




--- Comment #4 from Alexandru Chiculita <achicu at adobe.com>  2013-10-24 11:22:03 PST ---
(From update of attachment 214953)
View in context: https://bugs.webkit.org/attachment.cgi?id=214953&action=review

Thanks Mihnea. I have a couple comments below.

> Source/WebCore/rendering/RenderTreeAsText.cpp:664
> +static void writeRenderNamedFlowRegionList(const RenderRegionList& flowThreadRegionList, TextStream& ts, int indent)

I guess that the previous name was better in case we need to reuse the function for other types of RenderFlowThreads.

> Source/WebCore/rendering/RenderTreeAsText.cpp:667
> +        RenderNamedFlowFragment* region = toRenderNamedFlowFragment(*itRR);

There's no need to cast to RenderNamedFlowFragment.

> Source/WebCore/rendering/RenderTreeAsText.cpp:669
> +        ts << "RenderNamedFlowFragment";

We can use the renderer name function instead.

> Source/WebCore/rendering/RenderTreeAsText.cpp:670
> +        if (region->generatingElement()) {

region->generatingElement() is used a lot of times. Might be better to put it on the stack.

> Source/WebCore/rendering/RenderTreeAsText.cpp:717
>          if (!validRegionsList.isEmpty() || !invalidRegionsList.isEmpty()) {

You can remove this "if" statement. The condition is duplicated in the nested ifs.

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