1594003 - KeyboardEvent.repeat is always false on X11 if MOZ_USE_XINPUT2 is 1

Status: NEW

(Core :: Widget: Gtk, defect, P3)

Description

[Dahan Gong]

Attachment: lots-of-InitKeyEvent-on-Fedora+X11-version-of-Firefox.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/78.0.3904.87 Safari/537.36

Steps to reproduce:

As said in https://github.com/philc/vimium/issues/3386#issuecomment-529352537 and https://github.com/philc/vimium/issues/3386#issuecomment-549184566 , Firefox may dispatch lots of keydown events with .repeat=false during a long pressing.

Actual results:

There're lots keydown events and their .repeat are all false.

Tested environment is the apt-installed Firefox on Fedora 31 (Nvidia, X11). But as said by the user, this bug has not been reproduced on a fresh Firefox downloaded from Mozilla. And I've confirmed that the user's Firefox is using X11 window platform after reading the log.

Expected results:

The expected is, a first keydown event has a repeat of false and the following have .repeat = true.

Extra info:

• My analysis is in https://github.com/philc/vimium/issues/3386#issuecomment-549817478 .
• the attached image shows lots of InitKeyEvent items but no FilterEvents ones
  • InitXKBExtension has been Succeeded
  • time points of those InitKeyEvents are: 222626005, 222626505, 222626535, 222626568 and so on.

Comment 1

[Dahan Gong]

I've tested the Fedora version of Firefox 69.0.1 (https://apps.fedoraproject.org/packages/firefox) , and it still has such a problem even when I export MOZ_DISABLE_WAYLAND=1 to force it to run with X11).

I also found that its source code can be downloaded as firefox-69.0.1-4.fc31.src.rpm from https://koji.fedoraproject.org/koji/buildinfo?buildID=1388756 , so I downloaded it and compared the patched code with the official version. But the Fedora version also runs InitXKBExtension successfully and FilterEvents is also installed.

I have no idea about what's wrong with the Fedora version.

Comment 2

[Dahan Gong]

I've found the root cause of this bug - Fedora forces MOZ_USE_XINPUT2 to be 1 in its /usr/bin/firefox (a shell script to run Firefox), and if I run MOZ_USE_XINPUT2=0 /usr/lib64/firefox/firefox, then repeated keys will be recognized correctly.

Comment 3

[Sebastian Padilla]

Hi Dahan

Thanks for submitting this bug to us. I've tried to reproduce it using Firefox Nightly 72.0a1 (2019-11-14) (64-bit) on Windows 10 pro and wasn't able to reproduce it.

Does this issue occur with a fresh profile? You can find the steps here: https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager

Can you please download Firefox Nightly from here: https://nightly.mozilla.org/ and retest the problem and see if the issue still occurs there as well?

Thanks!

Sebastian

Comment 4

[Dahan Gong]

This issue is about X11 (X Window System, a client/server platform to draw graphics, which is used widely on Linux) and MOZ_USE_XINPUT2, and I tested it on Fedora and Pop!_OS.

Firefox for Windows uses Windows APIs to operate graphics, and it doesn't use X11, so this bug is not related with Windows 10.

I've installed Firefox Nightly 72.0a1.en-US.linux-x86_64.tar.gz2, and this bug is still there, when the environment variable of MOZ_USE_XINPUT2 is 1.

The root cause is, when MOZ_USE_XINPUT2 is 1, Firefox skips gdk_disable_multidevice() (https://dxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3750) and then some inner logic about underlying keyboard events gets broken.

I've searched in code about logging and found two potential causes:

• XGetKeyboardControl writes wrong data into mKeyboardState.auto_repeats;
• or the callback named FilterEvents (which is passed to gdk_window_add_filter) never gets called

But I can not recognize which one is the real reason.

And a developer may see https://github.com/philc/vimium/issues/3386#issuecomment-549817478 , there're some detailed discussions.

Comment 5

[Florencia Di Ciocco, NI to vbalducci]

Hi Dahan,

I wasn't able to reproduce the bug but I've chosen a component for this bug in hope that someone with more expertise may look at it. We'll await their answer. If you consider that there's another component that's more proper for this case you may change it.

Regards, Flor.

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Moving all open Keyboard/IME handling bugs to DOM: UI Events & Focus Handling component.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 8

[atbjyk]

It affected me on wayland too.
and gdk_window_add_filter is not supported on wayland and lacks upstream gtk anymore.
I trying my hack and seems working, but I may be missing something.

diff --git a/widget/gtk/nsGtkKeyUtils.cpp b/widget/gtk/nsGtkKeyUtils.cpp
index f4c7030a37c2..31f8f12fd6e6 100644
--- a/widget/gtk/nsGtkKeyUtils.cpp
+++ b/widget/gtk/nsGtkKeyUtils.cpp
@@ -55,6 +55,8 @@ Time KeymapWrapper::sLastRepeatableKeyTime = 0;
 KeymapWrapper::RepeatState KeymapWrapper::sRepeatState =
     KeymapWrapper::NOT_PRESSED;
 
+char16_t previous_key = 0;
+
 static const char* GetBoolName(bool aBool) { return aBool ? "TRUE" : "FALSE"; }
 
 static const char* GetStatusName(nsEventStatus aStatus) {
@@ -1848,9 +1850,24 @@ void KeymapWrapper::InitKeyEvent(WidgetKeyboardEvent& aKeyEvent,
   aKeyEvent.mPluginEvent.Copy(*aGdkKeyEvent);
   aKeyEvent.mTime = aGdkKeyEvent->time;
   aKeyEvent.mNativeKeyEvent = static_cast<void*>(aGdkKeyEvent);
-  aKeyEvent.mIsRepeat =
-      sRepeatState == REPEATING &&
-      aGdkKeyEvent->hardware_keycode == sLastRepeatableHardwareKeyCode;
+
+  if (*(aKeyEvent.mKeyValue.get()) == previous_key) {
+      aKeyEvent.mIsRepeat = true;
+  } else {
+      aKeyEvent.mIsRepeat = false;
+      if (aKeyEvent.mMessage == eKeyPress) {
+          previous_key = *(aKeyEvent.mKeyValue.get());
+      }
+  }
+
+  if (aKeyEvent.mMessage == eKeyUp) {
+      aKeyEvent.mIsRepeat = false;
+      previous_key = 0;
+  }
+
+  //aKeyEvent.mIsRepeat =
+  //    sRepeatState == REPEATING &&
+  //    aGdkKeyEvent->hardware_keycode == sLastRepeatableHardwareKeyCode;
 
   MOZ_LOG(
       gKeymapWrapperLog, LogLevel::Info,

Comment 9

[Duane Johnson]

I'd like to confirm that this is affecting us on X11, with MOZ_USE_XINPUT2=1 as well. event.repeat is always false, even on repeat key events.

Comment 10

[runarberg]

This is affecting me on Ubuntu 22.04.3 LTS with Firefox 122.0.1 via Snap and MOZ_ENABLE_WAYLAND=1 keyboardEvent.repeat is always false.

Comment 11

[Makoto Kato [:m_kato]]

(In reply to runarberg from comment #10)

This is affecting me on Ubuntu 22.04.3 LTS with Firefox 122.0.1 via Snap and MOZ_ENABLE_WAYLAND=1 keyboardEvent.repeat is always false.

Could you file a new issue for Wayland? This bug is XINPUT2 issue.

Comment 13

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

deliver_key_event() from gdkdevice-wayland.c may be a guide how to implement it on Wayland. We need to check if the key can repeat by xkb_keymap_key_repeats() call (we don't set repeat for Shift for instance).

Delay/repeat time it's not exposed by Gtk, see get_key_repeat() at gdkdevice-wayland.c so we can use keyboard_handle_repeat_info() from nsGtkKeyUtils.cpp or guess it otherwise as get_key_repeat() does.

Then we perhaps need to compare the key event times if they fit the system delay/repeat ones and set mIsRepeat.
So the code snippet from comment 8 looks correct but we need to add key/time checks there.

Comment 14

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

(In reply to Makoto Kato [:m_kato] from comment #11)

(In reply to runarberg from comment #10)

This is affecting me on Ubuntu 22.04.3 LTS with Firefox 122.0.1 via Snap and MOZ_ENABLE_WAYLAND=1 keyboardEvent.repeat is always false.

Could you file a new issue for Wayland? This bug is XINPUT2 issue.

We don't need a wayland bug here. I guess we want to create a new patch which works with X11/XINPUT2 and Wayland too.