1470047 - [Wayland] Should handle all key modifiers

Status: RESOLVED FIXED

(Core :: Widget: Gtk, enhancement)

Description

[Takuro Ashie [:ashie]]

From bug1468670

Most of key modifiers aren't handled on Wayland since nsGtkKeyUtils::InitBySystemSettingsWayland() in widget/gtk/nsGtkKeyUtils.cpp isn't implemented yet.

To make it better as well as nsGtkKeyUtils::InitBySystemSettingsX11(), it should be implemented by using wl_keyboard & xkb (maybe).

Comment 1

[Martin Stránský [:stransky] (ni? me)]

I wonder if we can use GdkKeymap for that.

Comment 2

[Martin Stránský [:stransky] (ni? me)]

Attachment: Bug 1470047 - Implement InitBySystemSettingsWayland() and get key modifiers on Wayland,

Use wl_keyboard_listener and keymap event to get key mapping on Wayland. Weston simple-im.c example
is used as a reference implementation and actual key modifiers are derived from Wayland/GDK code from
gdkkeys-wayland.c.

Review commit: https://reviewboard.mozilla.org/r/256050/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/256050/

Comment 3

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8991058 [details]
Bug 1470047 - Implement InitBySystemSettingsWayland() and get key modifiers on Wayland,

https://reviewboard.mozilla.org/r/256050/#review262900

::: widget/gtk/nsGtkKeyUtils.cpp:524
(Diff revision 1)
> +         enum xkb_keymap_format, enum xkb_keymap_compile_flags))
> +        dlsym(RTLD_DEFAULT, "xkb_keymap_new_from_string");
> +
> +    struct xkb_context *xkb_context = sXkbContextNew(XKB_CONTEXT_NO_FLAGS);
> +    struct xkb_keymap *keymap =
> +    	 sXkbKeymapNewFromString(xkb_context,

hm, wrong indent here.

Comment 4

[Takuro Ashie [:ashie]]

Comment on attachment 8991058 [details]
Bug 1470047 - Implement InitBySystemSettingsWayland() and get key modifiers on Wayland,

https://reviewboard.mozilla.org/r/256050/#review263342

::: widget/gtk/nsGtkKeyUtils.cpp:460
(Diff revision 1)
> +    static auto sXkbKeymapModGetIndex =
> +        (xkb_mod_index_t (*)(struct xkb_keymap *, const char *))
> +        dlsym(RTLD_DEFAULT, "xkb_keymap_mod_get_index");
> +
> +    mModifierMasks[aModifierIndex] =
> +        (1 << sXkbKeymapModGetIndex(aKeymap, aModifierName));

I'm not sure but since xkb_keymap_mod_get_index() can return XKB_MOD_INVALID(0xffffffff), it may cause unexpected result.

Comment 5

[Martin Stránský [:stransky] (ni? me)]

Comment on attachment 8991058 [details]
Bug 1470047 - Implement InitBySystemSettingsWayland() and get key modifiers on Wayland,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/256050/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5e46c8d0ed58
Implement InitBySystemSettingsWayland() and get key modifiers on Wayland, r=ashie

Comment 7

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5e46c8d0ed58

Comment 8

[aidan]

This patch seems to cause issues on wlroots-based compositors.  I'm not entirely sure what is going wrong (or if it is an issue in firefox (although I asked in the #sway irc channel on freenode, and they seemed to think it was a firefox bug)), but I'll try to attach a WAYLAND_DEBUG log.  Let me know if you need more information, or if I should open up a separate bug.

Comment 9

[aidan]

Attachment: WAYLAND_DEBUG log

Comment 10

[Martin Stránský [:stransky] (ni? me)]

(In reply to aidan from comment #8)
> This patch seems to cause issues on wlroots-based compositors.  I'm not
> entirely sure what is going wrong (or if it is an issue in firefox (although
> I asked in the #sway irc channel on freenode, and they seemed to think it
> was a firefox bug)), but I'll try to attach a WAYLAND_DEBUG log.  Let me
> know if you need more information, or if I should open up a separate bug.

What issues exactly? Can you file a new bug for it please?

Comment 11

[aidan]

Sorry about that.  Thought I included more information.  Anyways, I added a new bug here: https://bugzilla.mozilla.org/show_bug.cgi?id=1475802

Thanks.