[webkit-reviews] review denied: [Bug 117199] [CSSRegions] Improve getRegionsByContent by testing in flow thread coordinates instead of view coordinates : [Attachment 203702] Patch

[bugzilla-daemon at webkit.org]

Alexandru Chiculita <achicu at adobe.com> has denied Mihnea Ovidenie
<mihnea at adobe.com>'s request for review:
Bug 117199: [CSSRegions] Improve getRegionsByContent by testing in flow thread
coordinates instead of view coordinates
https://bugs.webkit.org/show_bug.cgi?id=117199

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

------- Additional Comments from Alexandru Chiculita <achicu at adobe.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=203702&action=review


> Source/WebCore/rendering/RenderFlowThread.cpp:818
> +    LayoutRect boundingBox;

Why not just use localToContainerQuad instead (now that it can also return
coordinates in the flowthread space)?

> Source/WebCore/rendering/RenderFlowThread.cpp:824
> +	   ASSERT(false);

nit: this could be ASSERT_NOT_REACHED.

Why only renderInlines and text? Looks like the caller could send any node in
here.

> Source/WebKit2/Shared/WebPreferencesStore.h:98
> +    macro(RegionBasedColumnsEnabled, regionBasedColumnsEnabled, Bool, bool,
true) \

I don't see any comments in the changelog about this change.