Suppress duplicated key event handling in Linux/GTK3 #4199#4200
Suppress duplicated key event handling in Linux/GTK3 #4199#4200fedejeanne wants to merge 1 commit into
Conversation
Test Results 855 files ±0 855 suites ±0 59m 26s ⏱️ + 8m 48s For more details on these failures, see this check. Results for commit 319c5cd. ± Comparison against base commit 5fe0fc9. ♻️ This comment has been updated with latest results. |
iloveeclipse
left a comment
There was a problem hiding this comment.
There are some test fails btw...
|
Beside test failure, the patch seem to work. Tried on RHEL 9.6 / X11 |
native key event (Linux/GTK) On GTK (Linux), a single physical key press can, in some situations, be delivered to KeyBindingDispatcher twice for the very same native event: once synchronously via an SWT.Traverse event dispatched from within the native gtk3_key_press_event handling, and once more later via the async message queue. Both deliveries resolve to and execute the exact same bound command, which for page-traversal shortcuts like Ctrl+PageUp / Ctrl+PageDown (Next/Previous Editor) causes an extra tab to be skipped, since the CTabFolder selection is advanced twice per key press. This does not reproduce on Windows or macOS, only on GTK. Fix: track the last executed command together with the native timestamp (Event#time) of its triggering event in executeCommand(...), and skip (without re-executing the handler) a call that matches both the same command and the same event timestamp, since that combination can only happen when the very same native key event is redelivered. Genuinely distinct key presses - including fast auto-repeat - always carry different timestamps, so normal navigation and repeated shortcuts are unaffected. This is reported as the 'bonus' issue in eclipse-platform#4135 (Javadoc View tab cycling two tabs per Ctrl+PageUp/PageDown on Linux). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e66fdd7 to
319c5cd
Compare
Reproducible locally, the test was added via 172b704 for https://bugs.eclipse.org/bugs/show_bug.cgi?id=294509, unfortunately bugzilla is down ATM. |
|
OK, the test is supposed to test this:
|
|
I am looking into it. I suspect something is wrong with the tests because even though they only fail when applying this PR, I also noticed that:
This smells like a bad state after a test ran. |
Fixes #4199
How to test
Navigate the views with
Ctrl + PgUp/Ctrl + PgDnlike I did in the video in #4199, no view should be skipped.