opendev/gerrit
Code Issues Proposed changes

Fix improper implementation of getKeyboardEvent

Browse Source 
Bug: Issue 5088
Change-Id: Iee82c135230f78f823c25982aaeba1ea6ece1709
This commit is contained in:
Kasper Nilsson
2016-12-07 18:02:34 -08:00
parent 62f911698d
commit d01ad017e4
11 changed files with 40 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html → polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
View File

@@ -13,20 +13,22 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="../bower_components/iron-a11y-keys-behavior/iron-a11y-keys-behavior.html">
<link rel="import" href="../../bower_components/polymer/polymer.html">
<link rel="import" href="../../bower_components/iron-a11y-keys-behavior/iron-a11y-keys-behavior.html">
<script>
(function(window) {
'use strict';
var getKeyboardEvent = function(e) {
return Polymer.dom(e.detail ? e.detail.keyboardEvent : e).event;
return Polymer.dom(e.detail ? e.detail.keyboardEvent : e);
};
var KeyboardShortcutBehaviorImpl = {
modifierPressed: function(e) {
e = getKeyboardEvent(e);
// When e is a keyboardEvent, e.event is not null.
if (e.event) { e = e.event; }
return e.altKey || e.ctrlKey || e.metaKey || e.shiftKey;
},

26
polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html → polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior_test.html
View File

@@ -42,6 +42,7 @@ limitations under the License.
suite('keyboard-shortcut-behavior tests', function() {
var element;
var overlay;
var sandbox;
suiteSetup(function() {
// Define a Polymer element that uses this behavior.
@@ -58,6 +59,11 @@ limitations under the License.
setup(function() {
element = fixture('basic');
overlay = fixture('within-overlay');
sandbox = sinon.sandbox.create();
});
teardown(function() {
sandbox.restore();
});
test('doesn’t block kb shortcuts for non-whitelisted els', function(done) {
@@ -101,5 +107,25 @@ limitations under the License.
MockInteractions.keyDownOn(divEl, 75, null, 'k');
});
test('modifierPressed returns accurate values', function() {
var spy = sandbox.spy(element, 'modifierPressed');
element._handleKey = function(e) {
element.modifierPressed(e);
};
MockInteractions.keyDownOn(element, 75, 'shift', 'k');
assert.isTrue(spy.lastCall.returnValue);
MockInteractions.keyDownOn(element, 75, null, 'k');
assert.isFalse(spy.lastCall.returnValue);
MockInteractions.keyDownOn(element, 75, 'ctrl', 'k');
assert.isTrue(spy.lastCall.returnValue);
MockInteractions.keyDownOn(element, 75, null, 'k');
assert.isFalse(spy.lastCall.returnValue);
MockInteractions.keyDownOn(element, 75, 'meta', 'k');
assert.isTrue(spy.lastCall.returnValue);
MockInteractions.keyDownOn(element, 75, null, 'k');
assert.isFalse(spy.lastCall.returnValue);
MockInteractions.keyDownOn(element, 75, 'alt', 'k');
assert.isTrue(spy.lastCall.returnValue);
});
});
</script>

2
polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.html
View File

@@ -15,7 +15,7 @@ limitations under the License.
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
<link rel="import" href="../../../styles/gr-change-list-styles.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">

2
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
View File

@@ -16,7 +16,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
<link rel="import" href="../../shared/gr-select/gr-select.html">

2
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
View File

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">

2
polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.html
View File

@@ -15,7 +15,7 @@ limitations under the License.
-->
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../shared/gr-autocomplete/gr-autocomplete.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">

2
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
View File

@@ -17,7 +17,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../bower_components/iron-dropdown/iron-dropdown.html">
<link rel="import" href="../../../behaviors/gr-url-encoding-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-overlay/gr-overlay.html">

2
polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
View File

@@ -15,7 +15,7 @@ limitations under the License.
-->
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<dom-module id="gr-selection-action-box">
<template>

2
polygerrit-ui/app/elements/gr-app.html
View File

@@ -15,7 +15,7 @@ limitations under the License.
-->
<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../styles/app-theme.html">
<link rel="import" href="./core/gr-error-manager/gr-error-manager.html">

2
polygerrit-ui/app/elements/shared/gr-button/gr-button.html
View File

@@ -16,7 +16,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-tooltip-behavior/gr-tooltip-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<dom-module id="gr-button">
<template strip-whitespace>

1
polygerrit-ui/app/test/index.html
View File

@@ -110,6 +110,7 @@ limitations under the License.
'gr-patch-set-behavior/gr-patch-set-behavior_test.html',
'gr-path-list-behavior/gr-path-list-behavior_test.html',
].forEach(function(file) {
// Behaviors do not utilize the DOM, so no shadow DOM test is necessary.
file = behaviorsPath + file;
testFiles.push(file);
});