1468670 - [Wayland] Alt modifier key is not handled on wayland

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Takuro Ashie [:ashie]]

From https://bugzilla.redhat.com/show_bug.cgi?id=1470031

Description of problem:

  When running firefox(cairo-gtk3-wayland) on wayland, Alt modifier key is not handled.

Steps to Reproduce:

  1. Launch Firefox
  2. Visit some pages
  3. Press Alt + ← or Alt + →

Expected results:

  Fire "GoForward" or "GoBack" event, then moving visited pages which listed on history in current tab.

Actual results:

  Nothing happens

Additional info:

  Firefox handles modifier keys by itself.
  It depends on X11.
  And when not existing X11, mModifierMasks is always set as 0.

    * https://dxr.mozilla.org/mozilla-central/rev/75a32b57132f8cba42779555662a057a0416a313/widget/gtk/nsGtkKeyUtils.cpp#
    * https://dxr.mozilla.org/mozilla-central/rev/75a32b57132f8cba42779555662a057a0416a313/widget/gtk/nsGtkKeyUtils.cpp#433

Comment 1

[Takuro Ashie [:ashie]]

It's reproduced on GNOME Shell & Weston.

Comment 2

[Takuro Ashie [:ashie]]

(In reply to Takuro Ashie from comment #0)
>     *
> https://dxr.mozilla.org/mozilla-central/rev/
> 75a32b57132f8cba42779555662a057a0416a313/widget/gtk/nsGtkKeyUtils.cpp#

Sorry, the line number is missing:

https://dxr.mozilla.org/mozilla-central/rev/75a32b57132f8cba42779555662a057a0416a313/widget/gtk/nsGtkKeyUtils.cpp#176

Comment 3

[Takuro Ashie [:ashie]]

Attachment: Bug 1468670 - Enablel Alt modifier key on Wayland

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

Comment 4

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

Thanks, I'll look at it.

Comment 5

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

Comment on attachment 8985299 [details]
Bug 1468670 - Enablel Alt modifier key on Wayland

https://reviewboard.mozilla.org/r/250924/#review257456

Looks good but please move the Wayland setup to a dedicated routine - see commend below. Thanks!

::: widget/gtk/nsGtkKeyUtils.cpp:198
(Diff revision 1)
>  
>      mModifierKeys.Clear();
>      memset(mModifierMasks, 0, sizeof(mModifierMasks));
>  
>      if (GDK_IS_X11_DISPLAY(gdk_display_get_default()))
>          InitBySystemSettings();

Please remove it to InitBySystemSettingsX11 (or similar) to stress that it's X11 only.

::: widget/gtk/nsGtkKeyUtils.cpp:200
(Diff revision 1)
>      memset(mModifierMasks, 0, sizeof(mModifierMasks));
>  
>      if (GDK_IS_X11_DISPLAY(gdk_display_get_default()))
>          InitBySystemSettings();
> +    else
> +        mModifierMasks[INDEX_ALT] = 1 << 3;

Please create a new method (InitBySystemSettingsWayland() or so) and move the init here. I guess we'd need more key settings so we can configure all there.

Comment 6

[Takuro Ashie [:ashie]]

Comment on attachment 8985299 [details]
Bug 1468670 - Enablel Alt modifier key on Wayland

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

Comment 7

[Takuro Ashie [:ashie]]

Comment on attachment 8985299 [details]
Bug 1468670 - Enablel Alt modifier key on Wayland

https://reviewboard.mozilla.org/r/250924/#review258132

Comment 8

[Takuro Ashie [:ashie]]

Comment on attachment 8985299 [details]
Bug 1468670 - Enablel Alt modifier key on Wayland

https://reviewboard.mozilla.org/r/250924/#review257456

> Please create a new method (InitBySystemSettingsWayland() or so) and move the init here. I guess we'd need more key settings so we can configure all there.

Done.

To handle it properly, probably we need to use wl_keyboard and xkb.
I'll file a new bug for it after this patch is merged.

Comment 9

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

Comment on attachment 8985299 [details]
Bug 1468670 - Enablel Alt modifier key on Wayland

https://reviewboard.mozilla.org/r/250924/#review258138

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/98620eaa1b5c
Enablel Alt modifier key on Wayland r=stransky

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/98620eaa1b5c

Comment 12

[Takuro Ashie [:ashie]]

(In reply to Takuro Ashie [:ashie] from comment #8)
> To handle it properly, probably we need to use wl_keyboard and xkb.
> I'll file a new bug for it after this patch is merged.

bug1470047

Comment 13

[Yo'av Moshe]

I'm also seeing this on Firefox 66.0b6 developer edition under Wayland, build 20190208180655.

It seems to effect this too: bug1511973

Comment 14

[tormen]

:ashie: I am having seemingly the same problem, as this one was closed, I thought it's better to refile: #1538052