[webkit-reviews] review granted: [Bug 117407] [CSS Regions] Fix getClientRects() for content nodes : [Attachment 230311] Patch

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has granted Andrei Bucur <abucur at adobe.com>'s
request for review:
Bug 117407: [CSS Regions] Fix getClientRects() for content nodes
https://bugs.webkit.org/show_bug.cgi?id=117407

Attachment 230311: Patch
https://bugs.webkit.org/attachment.cgi?id=230311&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=230311&action=review


r=me

> Source/WebCore/rendering/RenderNamedFlowThread.cpp:861
> +	   LayoutRect layoutLocalRect(0, localTop,
renderer->borderBoxRectInRegion(region).width(), localBottom - localTop);
> +	   LayoutRect fragmentRect = region->rectFlowPortionForBox(renderer,
layoutLocalRect);
> +
> +	   // We want to skip the 0px height fragments for non-empty boxes that
may appear in case the bottom of the box
> +	   // overlaps the bottom of a region.
> +	   if (localBottom == localTop || fragmentRect.height()) {
> +	       CurrentRenderRegionMaintainer regionMaintainer(*region);
> +	      
quads.append(renderer->localToAbsoluteQuad(FloatRect(fragmentRect), 0 /* mode
*/, wasFixed));
> +	   }

In my opinion, all of this could move to a virtual function on RenderRegion
that takes the quads as a reference argument. Then you could keep your code
exactly the same, but for region sets I could just add directly to the quads
list myself.