[webkit-qt] API review, comments needed!

Kent Hansen kent.hansen at nokia.com
Tue Mar 9 05:33:13 PST 2010


Hi Joseph,

ext Joseph Ligman wrote:
> Hi,
>
> I started working on the issues with QWebFrame::scrollRecursively, and  
> am looking for some suggestions.
>
> This API was introduced because the clients of QWebFrame were not able  
> to scroll element's with a CSS overflow property. Initially, I tried  
> to implement QWebElement::scroll to fix this, but reviewer's didn't  
> like that idea.  ScrollRecursively seemed to sound ok at that time.
>
> The hit test position using the last mouse position is  a bug.  For  
> this to work, the client has to continually update the mouse position  
> while scrolling and this can produce unexpected behavior.  I've been  
> planning on adding this position as a parameter to the new API, does  
> anyone have any other ideas?
>   

As Simon said, adding the position as a parameter was what we thought 
made sense in the API review too, so I'd suggest following that route.

> Since the name is confusing, I was thinking about changing this to  
> bool QWebFrame::ScrollFrameWithParent(int dx, int dy, QPoint  
> overflowHitTestPosition);  and also, I added the bool return type so  
> the client could easily check if scrolling happened, but I'm not sure  
> this is necessary (if we keep the bool fixing the documentation is no  
> problem)
>   

Yeah, the name is tricky. The only analogy I can think of so far is the 
concept of "event bubbling" in DOM (event goes from inner to outer 
element). scrollWithBubbling() or scrollBubblingly()?? :D

The bool return value can stay if there's a valid use case for having 
it. Why doesn't the existing scroll() function have a bool return value 
(you mentioned starting out by attempting to modify that function)? If 
an API user really wanted to know whether a scroll happened, couldn't he 
check some other property/ies after calling scrollXXX()? And instead of 
a bool, shouldn't it rather be the actual delta that was scrolled (if, 
say, 16px was requested but only 10px was possible)? In lack of further 
evidence, I'm in favor of dropping it.

Regards,
Kent

Kent



More information about the webkit-qt mailing list