[webkit-reviews] review granted: [Bug 134568] Selection rects sent to ServicesOverlayController are wrong : [Attachment 234317] Patch v2 - Addressing both Ryosuke's and Tim's feedback
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jul 3 10:32:57 PDT 2014
Darin Adler <darin at apple.com> has granted Brady Eidson <beidson at apple.com>'s
request for review:
Bug 134568: Selection rects sent to ServicesOverlayController are wrong
https://bugs.webkit.org/show_bug.cgi?id=134568
Attachment 234317: Patch v2 - Addressing both Ryosuke's and Tim's feedback
https://bugs.webkit.org/attachment.cgi?id=234317&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=234317&action=review
> Source/WebCore/rendering/RenderTextLineBoxes.cpp:500
> + for (auto box = m_first; box; box = box->nextTextBox()) {
I like to use auto* in cases like this to emphasize the fact that it’s a
pointer. But auto is also OK.
> Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:139
> + LayoutUnit leftEdge, rightEdge;
> + bool hasLeftEdge = false, hasRightEdge = false;
Separate lines please. Also not sure we need these booleans. Could just nest
the code a bit and we wouldn’t need the locals.
> Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:147
> + rightEdge = gap.right().x() + gap.right().width();
Should be:
rightEdge = gap.right().maxX();
> Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:178
> + rects.resize(3);
Can use shrink here instead of resize; slightly more efficient.
> Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:204
> + LayoutRect left, right;
Separate lines please.
More information about the webkit-reviews
mailing list