From 2de67491916345a894ec24a00dbeef81c37f9282 Mon Sep 17 00:00:00 2001
From: Kasper Nilsson <kaspern@google.com>
Date: Tue, 24 Oct 2017 11:03:07 -0700
Subject: [PATCH] Remove iron-a11y-keys bindings from gr-button

The file-list now uses native event listening to handle `enter`
keypresses (see Issue 7277 for more context). Because gr-button still
used iron-a11y-keys bindings, the `enter` event that was caught and
cancelled by the button was different from the `enter` event that was
caught by the file-list.

Migrating to native event listening on gr-buttons allows the event to be
properly cancelled.

Bug: Issue 7470
Change-Id: I7998243ce16b5cb5e6d3f903f184d39d89a1090d
---
 .../app/elements/shared/gr-button/gr-button.js  | 17 ++++++++++-------
 .../shared/gr-button/gr-button_test.html        | 10 ++++++++++
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
index 911f49d6eb..ef5e489a6f 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js
@@ -46,6 +46,7 @@
     listeners: {
       tap: '_handleAction',
       click: '_handleAction',
+      keydown: '_handleKeydown',
     },
 
     behaviors: [
@@ -58,10 +59,6 @@
       tabindex: '0',
     },
 
-    keyBindings: {
-      'space enter': '_handleCommitKey',
-    },
-
     _handleAction(e) {
       if (this.disabled) {
         e.preventDefault();
@@ -76,9 +73,15 @@
       this.setAttribute('tabindex', disabled ? '-1' : this._enabledTabindex);
     },
 
-    _handleCommitKey(e) {
-      e.preventDefault();
-      this.click();
+    _handleKeydown(e) {
+      if (this.modifierPressed(e)) { return; }
+      e = this.getKeyboardEvent(e);
+      // Handle `enter`, `space`.
+      if (e.keyCode === 13 || e.keyCode === 32) {
+        e.preventDefault();
+        e.stopPropagation();
+        this.click();
+      }
     },
   });
 })();
diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html b/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
index de8b5b1c24..d78427bbca 100644
--- a/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button_test.html
@@ -67,6 +67,16 @@ limitations under the License.
         MockInteractions.pressAndReleaseKeyOn(element, key);
         assert.isTrue(tapSpy.calledOnce);
       });
+
+      test('dispatches no tap event with modifier on keycode ' + key, () => {
+        const tapSpy = sandbox.spy();
+        element.addEventListener('tap', tapSpy);
+        MockInteractions.pressAndReleaseKeyOn(element, key, 'shift');
+        MockInteractions.pressAndReleaseKeyOn(element, key, 'ctrl');
+        MockInteractions.pressAndReleaseKeyOn(element, key, 'meta');
+        MockInteractions.pressAndReleaseKeyOn(element, key, 'alt');
+        assert.isFalse(tapSpy.calledOnce);
+      });
     }
 
     suite('disabled', () => {