[Wayland] Should handle all key modifiers

RESOLVED FIXED in Firefox 63

Status

()

enhancement
RESOLVED FIXED
10 months ago
14 days ago

People

(Reporter: ashie, Assigned: stransky)

Tracking

(Blocks 2 bugs)

Trunk
mozilla63
Unspecified
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

10 months ago
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).
(Reporter)

Updated

10 months ago
Blocks: wayland
(Reporter)

Updated

10 months ago
Summary: [Wayland] Should handle all modifier keys → [Wayland] Should handle all key modifiers
(Assignee)

Comment 1

10 months ago
I wonder if we can use GdkKeymap for that.
Comment hidden (mozreview-request)
(Assignee)

Updated

10 months ago
Assignee: nobody → stransky
(Assignee)

Comment 3

10 months ago
mozreview-review
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.
(Reporter)

Comment 4

10 months ago
mozreview-review
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.
Attachment #8991058 - Flags: review?(ashie) → review+
Comment hidden (mozreview-request)

Comment 6

10 months ago
Pushed by stransky@redhat.com:
https://hg.mozilla.org/integration/autoland/rev/5e46c8d0ed58
Implement InitBySystemSettingsWayland() and get key modifiers on Wayland, r=ashie

Comment 7

10 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/5e46c8d0ed58
Status: NEW → RESOLVED
Last Resolved: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63

Comment 8

10 months ago
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

10 months ago
Posted file WAYLAND_DEBUG log
(Assignee)

Comment 10

10 months ago
(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?
(Assignee)

Updated

10 months ago
Flags: needinfo?(aidan)

Comment 11

9 months ago
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.
Flags: needinfo?(aidan)
(Assignee)

Updated

9 months ago
Blocks: 1475802
No longer depends on: 1475802
You need to log in before you can comment on or make changes to this bug.