[Webkit-unassigned] [Bug 122963] [CSSRegions] Display anonymous regions in DRT
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Oct 25 04:35:44 PDT 2013
https://bugs.webkit.org/show_bug.cgi?id=122963
--- Comment #6 from Mihnea Ovidenie <mihnea at adobe.com> 2013-10-25 04:34:29 PST ---
(In reply to comment #4)
> (From update of attachment 214953 [details])
> 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.
>
I kept the original name and i added a virtual method - dumpRegion - in RenderRegion and in RenderNamedFlowFragment. The method dumps the region properties in a TextStream.
> > Source/WebCore/rendering/RenderTreeAsText.cpp:667
> > + RenderNamedFlowFragment* region = toRenderNamedFlowFragment(*itRR);
>
> There's no need to cast to RenderNamedFlowFragment.
>
I wanted this cast in order to make sure that these regions are in fact RenderNamedFlowFragments. With the new approach, there is no need for that.
> > Source/WebCore/rendering/RenderTreeAsText.cpp:669
> > + ts << "RenderNamedFlowFragment";
>
> We can use the renderer name function instead.
>
Done.
> > 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.
>
Done, both in RenderRegion::dumpRegion and in RenderNamedFlowFragment::dumpRegion.
> > 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.
Right :)
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list