Bug 210642 - [GTK] Update for GdkKeymap API changes
Summary: [GTK] Update for GdkKeymap API changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Claudio Saavedra
URL:
Keywords:
Depends on:
Blocks: GTK4 210651
  Show dependency treegraph
 
Reported: 2020-04-17 02:04 PDT by Claudio Saavedra
Modified: 2020-04-19 06:51 PDT (History)
3 users (show)

See Also:


Attachments
Patch (11.82 KB, patch)
2020-04-17 02:07 PDT, Claudio Saavedra
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Claudio Saavedra 2020-04-17 02:04:38 PDT
[GTK] Update for GdkKeymap API changes
Comment 1 Claudio Saavedra 2020-04-17 02:07:28 PDT
Created attachment 396747 [details]
Patch
Comment 2 Adrian Perez 2020-04-17 08:39:53 PDT
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.
Comment 3 Claudio Saavedra 2020-04-17 08:45:14 PDT
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?
Comment 4 Adrian Perez 2020-04-17 08:47:26 PDT
(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.
Comment 5 EWS 2020-04-17 08:50:17 PDT
Committed r260255: <https://trac.webkit.org/changeset/260255>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 396747 [details].
Comment 6 Carlos Garcia Campos 2020-04-19 06:51:12 PDT
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.