[Webkit-unassigned] [Bug 68192] Add method to scroll current node to specific position in Chromium WebKit API

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Darin Fisher (:fishd, Google) <fishd at chromium.org>  2011-09-15 23:44:20 PST ---
(From update of attachment 107563)
View in context: https://bugs.webkit.org/attachment.cgi?id=107563&action=review

> Source/WebKit/chromium/public/WebView.h:179
> +    virtual void scrollFocusedNodeIntoRect(const WebRect& rect) { }

nit: we don't name parameters when the name doesn't add any information.  here the type name
is WebRect, and calling the variable "rect" does not add any information, so the name should be left out.

why did you choose to have the rect be in screen coordinates instead of window coordinates?
that seems like a fairly unusual choice given that most other APIs use window coordinates
(e.g., input events).

> Source/WebKit/chromium/src/WebViewImpl.cpp:1828
> +    LayoutRect bounds = elementNode->boundsInWindowSpace();

it seems like this requires layout() to have been run before calling
this function.  should you perhaps assert that layout is not dirty?

> Source/WebKit/chromium/src/WebViewImpl.cpp:1832
> +    Frame* frame = mainFrameImpl()->frame();

how do you know that the focused node is part of the main frame?
what if the focused node is part of a subframe?

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