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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 16 06:15:42 PDT 2011


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





--- Comment #11 from Varun Jain <varunjain at chromium.org>  2011-09-16 06:15:42 PST ---
Hi Darin,
Dimitri and I agreed to moving this code into FrameView to be able to test it with window.internals in another CL. This is the tracking bug: https://bugs.webkit.org/show_bug.cgi?id=68198


(In reply to comment #8)
> (From update of attachment 107563 [details])
> 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.
> 

will fix in followup CL

> 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).

No particular reason for this except that the renderer get the rect in screen coordinates. I can change it to window coordinates in the renderer before passing to webview

> 
> > 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?

Yes.. will check for dirty layout and relayout if required.

> 
> > 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?

I totally missed that! Thanks for pointing out. I do like Fady's suggestion above.

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



More information about the webkit-unassigned mailing list