[webkit-reviews] review denied: [Bug 110895] Focus ring for a child layer is incorrectly offset by ancestor composited layer's position : [Attachment 190636] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Feb 27 19:49:54 PST 2013
Simon Fraser (smfr) <simon.fraser at apple.com> has denied Xianzhu Wang
<wangxianzhu at chromium.org>'s request for review:
Bug 110895: Focus ring for a child layer is incorrectly offset by ancestor
composited layer's position
https://bugs.webkit.org/show_bug.cgi?id=110895
Attachment 190636: Patch
https://bugs.webkit.org/attachment.cgi?id=190636&action=review
------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=190636&action=review
The fix is good, just some naming quibbles.
> Source/WebCore/ChangeLog:31
> + (RenderBox):
You should remove useless lines like this.
> Source/WebCore/rendering/PaintInfo.h:58
> + OverlapTestRequestMap* overlapTestRequests = 0, const
RenderLayerModelObject* newRepaintContainer = 0)
The name is a bit confusing, since this isn't about "repaint". Let's call it
"paintContainer" here. Or we could rename "paintingRoot", and re-use that name
for this.
> Source/WebCore/rendering/RenderBlock.h:538
> + virtual void addFocusRingRects(Vector<IntRect>&, const LayoutPoint&,
const RenderLayerModelObject*);
This should name the parameter so it's easy to tell what it's for. Perhaps
"rootForRects" or "paintingRoot".
> Source/WebCore/rendering/RenderBox.h:165
> + virtual void addFocusRingRects(Vector<IntRect>&, const LayoutPoint&,
const RenderLayerModelObject*);
Ditto.
> Source/WebCore/rendering/RenderInline.cpp:1368
> +void RenderInline::addFocusRingRects(Vector<IntRect>& rects, const
LayoutPoint& additionalOffset, const RenderLayerModelObject* repaintContainer)
repaintContainer->paintingRoot.
> Source/WebCore/rendering/RenderInline.h:84
> + virtual void addFocusRingRects(Vector<IntRect>&, const LayoutPoint&,
const RenderLayerModelObject*);
Same here. If you're touching these functions, you might as well add OVERRIDE.
More information about the webkit-reviews
mailing list