[Webkit-unassigned] [Bug 132372] Phone number data detection UI is offset for iframes, pages with topContentInset

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=132372





--- Comment #7 from Darin Adler <darin at apple.com>  2014-04-30 09:28:44 PST ---
(From update of attachment 230452)
View in context: https://bugs.webkit.org/attachment.cgi?id=230452&action=review

> Source/WebKit2/WebProcess/WebPage/mac/TelephoneNumberOverlayControllerMac.mm:73
> +        Frame* frame = range->ownerDocument().frame();

The code below uses FrameView, not Frame, so I suggest putting that into a local variable. Also, I suggest using a reference, not a pointer.

> Source/WebKit2/WebProcess/WebPage/mac/TelephoneNumberOverlayControllerMac.mm:76
> +        IntPoint frameOffset(frame->view()->scrollOffset().width(), frame->view()->scrollOffset().height());
> +        offset.move(frameOffset.x(), frameOffset.y());

>From a micro-code-structure point of view, this can just be:

    offset.move(view.scrollOffset());

There’s no need to convert the IntSize to an IntPoint since IntPoint::move already takes an IntSize. That’s separate from the question you are discussing with Simon about whether this is OK.

I fact, again ignoring the correctness of the coordinate math that you are discussing with Simon, I think this 6-line snippet can be written like in two lines like this:

    FrameView& view = *range->ownerDocument().view();
    rect.setLocation(view.contentsToRootView(rect.location() + view.scrollOffset()));

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.