Bug 210642 – [GTK] Update for GdkKeymap API changes

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Claudio Saavedra]

[GTK] Update for GdkKeymap API changes

[Claudio Saavedra]

Created attachment 396747 [details]
Patch

[Adrian Perez]

Comment on attachment 396747 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396747&action=review

> Source/WebCore/platform/gtk/PlatformKeyboardEventGtk.cpp:1345
> +    return gdk_keymap_get_caps_lock_state(gdk_keymap_get_for_display(gdk_display_get_default()));

Ah, too bad we are not saving the “GdkEventKey“ as part of the
“PlatformKeyboardEvent” struct; if that was the case we could use

  gdk_keymap_get_for_display(gdk_window_get_display(gdk_event_get_window(event)))

to make this more precise. Anyhoo, just throwing ideas around, I think
the proposed change is fine as-is because the _get_default() was being
used before anyway.

> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:307
> +    GdkDisplay* display = gdk_window_get_display(pressEvent->key.window);

Please change this to use “gdk_event_get_window(pressEvent)” before
landing instead of accessing “pressEvent->key.window” directly.

[Claudio Saavedra]

Comment on attachment 396747 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396747&action=review

>> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:307
>> +    GdkDisplay* display = gdk_window_get_display(pressEvent->key.window);
> 
> Please change this to use “gdk_event_get_window(pressEvent)” before
> landing instead of accessing “pressEvent->key.window” directly.

There are other direct accesses to GdkEvent members, should we instead fix those in a follow up?

[Adrian Perez]

(In reply to Claudio Saavedra from comment #3)
> Comment on attachment 396747 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=396747&action=review
> 
> >> Tools/WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:307
> >> +    GdkDisplay* display = gdk_window_get_display(pressEvent->key.window);
> > 
> > Please change this to use “gdk_event_get_window(pressEvent)” before
> > landing instead of accessing “pressEvent->key.window” directly.
> 
> There are other direct accesses to GdkEvent members, should we instead fix
> those in a follow up?

Sure, a follow-up works as well.

[EWS]

Committed r260255: <https://trac.webkit.org/changeset/260255>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396747 [details].

[Carlos Garcia Campos]

Comment on attachment 396747 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=396747&action=review

>> Source/WebCore/platform/gtk/PlatformKeyboardEventGtk.cpp:1345
>> +    return gdk_keymap_get_caps_lock_state(gdk_keymap_get_for_display(gdk_display_get_default()));
> 
> Ah, too bad we are not saving the “GdkEventKey“ as part of the
> “PlatformKeyboardEvent” struct; if that was the case we could use
> 
>   gdk_keymap_get_for_display(gdk_window_get_display(gdk_event_get_window(event)))
> 
> to make this more precise. Anyhoo, just throwing ideas around, I think
> the proposed change is fine as-is because the _get_default() was being
> used before anyway.

We don't save the GdkKeyEvent, because this is created in the web process, and we can't send the GdkWindow of the WebKitWebView to the web process.