Summary: | [CSSRegions] getBoundingClientRect wrong for inline content nodes | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mihnea Ovidenie <mihnea> | ||||||||
Component: | CSS | Assignee: | Mihnea Ovidenie <mihnea> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achicu, commit-queue, esprehn+autocc, glenn, WebkitBugTracker | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Mihnea Ovidenie
2013-06-06 06:11:35 PDT
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. |