[Webkit-unassigned] [Bug 64790] [GTK][WK2] Handle doneWithKeyEvent in GTK port

[bugzilla-daemon at webkit.org]

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





--- Comment #17 from Martin Robinson <mrobinson at webkit.org>  2011-07-25 11:37:05 PST ---
(From update of attachment 101854)
View in context: https://bugs.webkit.org/attachment.cgi?id=101854&action=review

I'm a bit concerned about the interaction of key release and following key press events.

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:210
> +    if (!wasEventHandled) {

Please just use an early return here.

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:212
> +        webkitWebViewBaseSetForwardEvent(webkitWebViewBase);

With the way the code is written now, what happens when this is called on a key release event? Won't this setting then effect the behavior of the next key press?

> Source/WebKit2/UIProcess/API/gtk/PageClientImpl.cpp:213
> +        GdkEvent inputEvent = *event.nativeEvent();

It's probably better to use: GOwnPtr<GdkEvent> inputEvent(gdk_event_copy(event.nativeEvent())); here.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:214
> +    // Since WebProcess key event handling is not synchronous, handle the event in two passes.
> +    // When WebProcess processes the input event, it will call PageClientImpl::doneWithKeyEvent 
> +    // with event handled status which determines whether to pass the input event to parent or not 
> +    // using gtk_main_do_event().
> +    if (priv->shouldForwardKeyboardEvent) {
> +        priv->shouldForwardKeyboardEvent = FALSE;
> +        return GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->key_press_event(widget, event);
> +    }
>      priv->pageProxy->handleKeyboardEvent(NativeWebKeyboardEvent(reinterpret_cast<GdkEvent*>(event)));
> -
> -    return GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->key_press_event(widget, event);
> +    return TRUE;

I should clarify my confusion here. I do not understand how priv->shouldForwardKeyboardEvent can be true when webkitWebViewBaseKeyPressEvent is called. The only way I can see this happening is if during a previous key release event, priv->shouldForwardKeyboardEvent was set to true. I do not think previous key release events should affect future key press events (see my comment above).

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:339
> +void webkitWebViewBaseSetForwardEvent(WebKitWebViewBase* webkitWebViewBase)
> +{
> +    webkitWebViewBase->priv->shouldForwardKeyboardEvent = TRUE;
> +}

I think you can just eliminate this method entirely and do this directly in PageClientImpl::doneWithKeyEvent.

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