Bug 24103 – [GTK] Use correct return value for WebView button-release handler

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Xan Lopez]

We are returning whatever the core code tells us it did, but this does not play well with the GTK+ model. GTK+ in general expects it will see a button-release if it saw a button-press and no motion/leave/etc events in between. EventHandler.cpp will, in some cases, not handle press but handle release, confusing the parent container of the WebView.

As a workaround return always FALSE for button-release (this is the same than the Windows port does).

[Xan Lopez]

Created attachment 27892 [details]
releaseevent.patch

Fix return value for release-event handler.

[Xan Lopez]

Created attachment 27893 [details]
releaseevent.patch

Slightly improve comment, don't confuse event with click.

[Christian Dywan]

> Slightly improve comment, don't confuse event with click.

While I would of course prefer to actually fix the event handlers in WebCore that don't play together , I suppose this is a passable compromise.

We might want to document somewhere that WebKit won't ever catch button events but still process them. Maybe in the introductory section of WebKitWebView?

[Xan Lopez]

(In reply to comment #3)
> > Slightly improve comment, don't confuse event with click.
> 
> While I would of course prefer to actually fix the event handlers in WebCore
> that don't play together , I suppose this is a passable compromise.

I'm not sure there's anything to fix. The event model and semantics of GTK+ and WebCore do not necesarily need to work OK together in all cases, and it's our responsibility to act as proxies between them. For this case in particular, ap says WebCore is working as it should be, for example.

> 
> We might want to document somewhere that WebKit won't ever catch button events
> but still process them. Maybe in the introductory section of WebKitWebView?
> 

Yes, I guess this should be documented. Do you think this approach in general makes sense? We could do other things, like catch everything (return TRUE always for both events), catch nothing (return FALSE always for both events)...

[Christian Dywan]

(In reply to comment #4)
> (In reply to comment #3)
> > > Slightly improve comment, don't confuse event with click.
> > 
> > While I would of course prefer to actually fix the event handlers in WebCore
> > that don't play together , I suppose this is a passable compromise.
> 
> I'm not sure there's anything to fix. The event model and semantics of GTK+ and
> WebCore do not necesarily need to work OK together in all cases, and it's our
> responsibility to act as proxies between them. For this case in particular, ap
> says WebCore is working as it should be, for example.

Yes, I'm not suggesting to change WebCore. If anything, we could try to make sure that a handled button-press is always followed by a handled button-release, i.e. just keep track of what button-press did and do the same in button-release.

> > We might want to document somewhere that WebKit won't ever catch button events
> > but still process them. Maybe in the introductory section of WebKitWebView?
> 
> Yes, I guess this should be documented. Do you think this approach in general
> makes sense? We could do other things, like catch everything (return TRUE
> always for both events), catch nothing (return FALSE always for both events)...

I would think what I described above could be better. If that doesn't work for whatever reason, always returning FALSE is probably the best we can do. 

[Xan Lopez]

(In reply to comment #5)
> > I'm not sure there's anything to fix. The event model and semantics of GTK+ and
> > WebCore do not necesarily need to work OK together in all cases, and it's our
> > responsibility to act as proxies between them. For this case in particular, ap
> > says WebCore is working as it should be, for example.
> 
> Yes, I'm not suggesting to change WebCore. If anything, we could try to make
> sure that a handled button-press is always followed by a handled
> button-release, i.e. just keep track of what button-press did and do the same
> in button-release.

Right, I thought about that too, but I think I prefer trying this first and see if it's enough, like you do.

[Alexey Proskuryakov]

Comment on attachment 27893 [details]
releaseevent.patch

Looks reasonable, r=me

[Holger Freyther]

Landed in r41248.