From 289374a43c1b2fd715ddf7455db225b17afebbaf Mon Sep 17 00:00:00 2001 From: Eirik Blakstad Date: Fri, 16 Nov 2018 14:01:32 +0100 Subject: [PATCH] fix(aria/ngClick): check if element is `contenteditable` before blocking spacebar `ngAria`'s `ngClick` blocks spacebar keypresses on non-blacklisted elements, which is an issue when the element is `contenteditable`. Closes #16762 --- src/ngAria/aria.js | 2 +- test/ngAria/ariaSpec.js | 129 +++++++++++++++++++++++++++++++++------- 2 files changed, 110 insertions(+), 21 deletions(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index cc7e481c5588..e3bfebd1c8b6 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -390,7 +390,7 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { if (keyCode === 13 || keyCode === 32) { // If the event is triggered on a non-interactive element ... - if (nodeBlackList.indexOf(event.target.nodeName) === -1) { + if (nodeBlackList.indexOf(event.target.nodeName) === -1 && !event.target.isContentEditable) { // ... prevent the default browser behavior (e.g. scrolling when pressing spacebar) // See https://github.com/angular/angular.js/issues/16664 event.preventDefault(); diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index e36fb9cc2778..2f96cb2f0a0a 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -954,6 +954,115 @@ describe('$aria', function() { expect(clickEvents).toEqual(['div(true)', 'li(true)', 'div(true)', 'li(true)']); }); + it('should trigger a click in browsers that provide `event.which` instead of `event.keyCode`', + function() { + compileElement( + '
' + + '
' + + '' + + '
'); + + var divElement = element.find('div'); + var liElement = element.find('li'); + + divElement.triggerHandler({type: 'keydown', which: 13}); + liElement.triggerHandler({type: 'keydown', which: 13}); + divElement.triggerHandler({type: 'keydown', which: 32}); + liElement.triggerHandler({type: 'keydown', which: 32}); + + expect(clickEvents).toEqual(['div(true)', 'li(true)', 'div(true)', 'li(true)']); + } + ); + + it('should not prevent default keyboard action if the target element has editable content', + inject(function($document) { + // Note: + // `contenteditable` is an enumarated (not a boolean) attribute (see + // https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/contenteditable). + // We need to check the following conditions: + // - No attribute. + // - Value: "" + // - Value: "true" + // - Value: "false" + + function eventFor(keyCode) { + return {bubbles: true, cancelable: true, keyCode: keyCode}; + } + + compileElement( + '
' + + // No attribute. + '
' + + '
' + + '
' + + '
' + + + // Value: "" + '
' + + '
' + + '
' + + '
' + + + // Value: "true" + '
' + + '
' + + '
' + + '
' + + + // Value: "false" + '
' + + '
' + + '
' + + '
' + + '
'); + + // Support: Safari 11-12+ + // Attach to DOM, because otherwise Safari will not update the `isContentEditable` property + // based on the `contenteditable` attribute. + $document.find('body').append(element); + + var containers = element.children(); + var container; + + // Using `browserTrigger()`, because it supports event bubbling. + + // No attribute | Elements are not editable. + container = containers.eq(0); + browserTrigger(container.find('div'), 'keydown', eventFor(13)); + browserTrigger(container.find('ul'), 'keydown', eventFor(32)); + browserTrigger(container.find('li'), 'keydown', eventFor(13)); + + expect(clickEvents).toEqual(['div(true)', 'ul(true)', 'li(true)']); + + // Value: "" | Elements are editable. + clickEvents = []; + container = containers.eq(1); + browserTrigger(container.find('div'), 'keydown', eventFor(32)); + browserTrigger(container.find('ul'), 'keydown', eventFor(13)); + browserTrigger(container.find('li'), 'keydown', eventFor(32)); + + expect(clickEvents).toEqual(['div(false)', 'ul(true)', 'li(false)']); + + // Value: "true" | Elements are editable. + clickEvents = []; + container = containers.eq(2); + browserTrigger(container.find('div'), 'keydown', eventFor(13)); + browserTrigger(container.find('ul'), 'keydown', eventFor(32)); + browserTrigger(container.find('li'), 'keydown', eventFor(13)); + + expect(clickEvents).toEqual(['div(false)', 'ul(true)', 'li(false)']); + + // Value: "false" | Elements are not editable. + clickEvents = []; + container = containers.eq(3); + browserTrigger(container.find('div'), 'keydown', eventFor(32)); + browserTrigger(container.find('ul'), 'keydown', eventFor(13)); + browserTrigger(container.find('li'), 'keydown', eventFor(32)); + + expect(clickEvents).toEqual(['div(true)', 'ul(true)', 'li(true)']); + }) + ); + they('should not prevent default keyboard action if an interactive $type element' + 'is nested inside ng-click', nodeBlackList, function(elementType) { function createHTML(type) { @@ -981,26 +1090,6 @@ describe('$aria', function() { } ); - it('should trigger a click in browsers that provide `event.which` instead of `event.keyCode`', - function() { - compileElement( - '
' + - '
' + - '' + - '
'); - - var divElement = element.find('div'); - var liElement = element.find('li'); - - divElement.triggerHandler({type: 'keydown', which: 13}); - liElement.triggerHandler({type: 'keydown', which: 13}); - divElement.triggerHandler({type: 'keydown', which: 32}); - liElement.triggerHandler({type: 'keydown', which: 32}); - - expect(clickEvents).toEqual(['div(true)', 'li(true)', 'div(true)', 'li(true)']); - } - ); - they('should not bind to key events if there is existing `ng-$prop`', ['keydown', 'keypress', 'keyup'], function(eventName) { scope.onKeyEvent = jasmine.createSpy('onKeyEvent');