[Webkit-unassigned] [Bug 78097] [GTK] Add WebKitWebView::hovered-over-element signal to WebKit2 GTK+ API

Wed Feb 8 11:02:54 PST 2012

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

Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mrobinson at webkit.org

--- Comment #7 from Martin Robinson <mrobinson at webkit.org>  2012-02-08 11:02:54 PST ---
(In reply to comment #6)

> > The fact that the element is not one of the types of elements that a hit test specifically measures against is also interesting information. Additionally, in the future, the hit test may also contain the DOM node. 
> 
> In that case (which is unlikely to happen) wkHitTestResultIsEmpty won't return true, because it will contain a node. The same will happen if we add more info to HitTestResult in the C API, like whether it's over a selection or editable content.

I think it's important to send the HitTest even when it's empty. This doesn't make the logic in embedders any more complicated and protects against poorly written applications that assume the HitTest isn't NULL. In general, I think we should avoid forcing NULL checks in embedders when it makes sense.

Another thing is that this "feels" wrong to me. What does it mean when a hit test returns a NULL result? The idea that a hit test returns a result that says "I didn't hit anything" seems more sensible to me.  I think perhaps you are thinking of the HitTestResult as "items found by the hit test," whereas I think of it more like GAsyncResult.

> > Is it important to only send the first hover event?
> 
> Emitting the signal a lot of times with the same hit test result means apps would need to check whether it's the same or not.
> 
> > If it's because this causes a lot of CPU usage,
> 
> It's not only because the CPU usage, it makes the API easier to use. The signal is called hovered-over-element, I woulnd't expect it to be emitted twice for the same element. This the same behaviour of the webkit1 hovering-link signal, fwiw. 

I'm this case, I think this particular change (only emitting the signal once per element) is probably fine in the end. As long as a new HitTest is always sent when you stop hovering on an element, this seems safe.

> > I think we should just rate limit here than changing the behavior entirely.
> what behaviour are we changing?

We're changing the behavior from the WebKit2 C API. Part of the reason I'm so picky about some of this stuff is that we are going under the assumption that the C API could possibly be removed. If the C API is removed, we'll need to rewrite the WebKitTestRunner against the GObject API. If that happens, we may not be able to properly run all the tests if the API isn't rich enough. In this case, I think you've convinced me that it's a fairly safe change though.

> >  I dislike the idea of making this signal too different from the underlying C API signal before we know whether or not it's important.
> 
> I think it's important, try to implement the url hovering in minibrowser if this signal is emitted a lot of times for the same link. I dislike the idea of making this signal too different from the WebKit1 equivalent :-P

In WebKit1 it seems that the signal is emmitted every time ChromeClient::mouseDidMoveOverElement is called, so wouldn't this be a change? In any case, either behavior is probably fine here.

> >  I also think it's consistent. An empty hit test result is still a hit test result.
> 
> NULL is an empty hit test result. I don't want apps checking all the possible values of a hit test result object and all of the returning NULL.

Why not add a webkit_hit_test_is_empty method then? The apps can also check if context is 0. I think this is safer than having this value be null.

> > I think it makes sense to just extend WebViewTest with another fixture. If you add this the name "UIClientTest" is no longer strictly accurate.
> 
> mouseDidMoveOverElement is a callback of the UI Client. 

You're absolutely right.

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