Page MenuHomePhabricator

Gerrit breaks Firefox keyboard shortcuts
Closed, ResolvedPublic

Description

After the upgrade to Gerrit 2.12.2, I noticed that on the change screen, Ctrl+T does not open a new tab but rather is treated as the "Edit change topic" shortcut, which does not require Ctrl. The same is true for other keyboard shortcuts such as Ctrl+C and Ctrl+R.

This very annoying issue has resulted in numerous upstream bug reports:

It is fixed in 2.12.3 though.

Related Objects

StatusSubtypeAssignedTask
Resolveddemon
ResolvedNone

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 25 2016, 7:52 PM

This is deffintly fixed in gerrit 2.12.3, you can confirm it by going to http://gerrit-test.wmflabs.org/ please which runs gerrit 2.12.3

hashar triaged this task as Medium priority.Jul 25 2016, 8:51 PM
hashar moved this task from Backlog to Patch merged upstream on the Upstream board.
bd808 added a subscriber: bd808.Jul 25 2016, 9:18 PM

] and [ are broken for me as well once I'm on a diff screen (can't move from file to file). This might be https://bugs.chromium.org/p/gerrit/issues/detail?id=1207 upstream. Things do seem to work at https://gerrit-review.googlesource.com where they are running 2.12.3-3357-g959bad3.

There actually running master gerrit not gerrit 2.12.3, I found that out when testing.

But you should be able to accurately test on http://gerrit-test.wmflabs.org/

jayvdb added a subscriber: jayvdb.Jul 26 2016, 4:56 AM

This will take some getting used to. Ctrl-T has hit me a few times.
Not an UBN of course, but maybe priority High ?

If a full upgrade to 2.12.3 (released July 6) isnt possible in the short term ... ?

... the patch for that bug is pretty small and understandable. maybe it can be applied separately?

https://gerrit-review.googlesource.com/#/c/75987/3/gerrit-gwtexpui/src/main/java/com/google/gwtexpui/globalkey/client/KeyCommandSet.java

  static int toMask(final KeyPressEvent event) {
     int mask = event.getUnicodeCharCode();
     if (mask == 0) {
        mask = event.getNativeEvent().getKeyCode();
     }
+    if (event.isControlKeyDown()) {
+       mask |= KeyCommand.M_CTRL;
+    }
+    if (event.isMetaKeyDown()) {
+       mask |= KeyCommand.M_META;
+    }
     return mask;
  }
Paladox moved this task from Bugs & stuff to Maybe fixed? on the Gerrit board.Aug 5 2016, 4:08 PM
demon closed this task as Resolved.Aug 15 2016, 3:25 PM
demon claimed this task.
demon added a subscriber: demon.

Upgraded to 2.12.3 this should be fixed.