Bug 117290

Created attachment 203935 [details] Patch

Comment on attachment 203935 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203935&action=review Looks good. > LayoutTests/fast/regions/flowed-inline-content-bounding-client-rect.html:2 > + nit: I've usually been asked to make tests look like a standard html page with head & body tags. > Source/WebCore/rendering/RenderFlowThread.cpp:1157 > + RenderRegion* region = mapFromFlowToRegion(transformState); > + if (region) nit: You can combine these lines in one. > Source/WebCore/rendering/RenderFlowThread.cpp:1158 > + static_cast<const RenderObject*>(region)->mapLocalToContainer(region->containerForRepaint(), transformState, mode, wasFixed); nit: Mihai was right on the other bug. We need to fix this cast.

(In reply to comment #2) > (From update of attachment 203935 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203935&action=review > > Looks good. > > > LayoutTests/fast/regions/flowed-inline-content-bounding-client-rect.html:2 > > + > > nit: I've usually been asked to make tests look like a standard html page with head & body tags. > > > Source/WebCore/rendering/RenderFlowThread.cpp:1157 > > + RenderRegion* region = mapFromFlowToRegion(transformState); > > + if (region) > > nit: You can combine these lines in one. > > > Source/WebCore/rendering/RenderFlowThread.cpp:1158 > > + static_cast<const RenderObject*>(region)->mapLocalToContainer(region->containerForRepaint(), transformState, mode, wasFixed); > > nit: Mihai was right on the other bug. We need to fix this cast. Why? I am not a fan of making RFT friend for RenderRegion and also i am not a huge fan of adding mapLocalToContainer and make it public in RenderRegion. Both in RenderBox and RenderInline, the function is protected/private.

(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 203935 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=203935&action=review > > > > Looks good. > > > > > LayoutTests/fast/regions/flowed-inline-content-bounding-client-rect.html:2 > > > + > > > > nit: I've usually been asked to make tests look like a standard html page with head & body tags. > > > > > Source/WebCore/rendering/RenderFlowThread.cpp:1157 > > > + RenderRegion* region = mapFromFlowToRegion(transformState); > > > + if (region) > > > > nit: You can combine these lines in one. > > > > > Source/WebCore/rendering/RenderFlowThread.cpp:1158 > > > + static_cast<const RenderObject*>(region)->mapLocalToContainer(region->containerForRepaint(), transformState, mode, wasFixed); > > > > nit: Mihai was right on the other bug. We need to fix this cast. > > Why? I am not a fan of making RFT friend for RenderRegion and also i am not a huge fan of adding mapLocalToContainer and make it public in RenderRegion. Both in RenderBox and RenderInline, the function is protected/private. Yeah. I'm not a fan of that either.

Created attachment 204006 [details] Patch for landing

Comment on attachment 204006 [details] Patch for landing Rejecting attachment 204006 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 204006, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Alex Chiculita found in /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog does not appear to be a valid reviewer according to committers.py. /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.appspot.com/results/716693

Created attachment 204007 [details] Patch for landing

Comment on attachment 204007 [details] Patch for landing Clearing flags on attachment: 204007 Committed r151309: <http://trac.webkit.org/changeset/151309>

All reviewed patches have been landed. Closing bug.