From 4be80aefa2617ac16b849dd8000dcfab39b5bb1e Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Sat, 1 Sep 2018 11:14:20 +0200 Subject: [PATCH 1/3] fix(ngAria.ngClick): restrict preventDefault on space / enter to non-interactive elements Fixes #16664 --- src/ngAria/aria.js | 8 ++++++-- test/ngAria/ariaSpec.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index 73e9e0733f9f..82216bfb464c 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -389,8 +389,12 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { var keyCode = event.which || event.keyCode; if (keyCode === 13 || keyCode === 32) { - // Prevent the default browser behavior (e.g. scrolling when pressing spacebar). - event.preventDefault(); + // Prevent the default browser behavior (e.g. scrolling when pressing spacebar) ... + if (nodeBlackList.indexOf(event.target.nodeName) === -1) { + // ... but only when the event is triggered by a non-interactive element + // See https://github.com/angular/angular.js/issues/16664 + event.preventDefault(); + } scope.$apply(callback); } diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index 11130b17430e..36f785e82edd 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -1,5 +1,7 @@ 'use strict'; +/* globals nodeBlackList false */ + describe('$aria', function() { var scope, $compile, element; @@ -952,6 +954,34 @@ describe('$aria', function() { expect(clickEvents).toEqual(['div(true)', 'li(true)', 'div(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) { + var html = '<' + type + '>'; + + if (type === 'INPUT' || 'TYPE' === 'A') { + return html; + } + + return html + ''; + } + + compileElement( + '
' + + '
' + createHTML(elementType) + '
' + + '
'); + + var divElement = element.find('div'); + var interactiveElement = element.find(elementType); + + // Use browserTrigger because it supports event bubbling + browserTrigger(interactiveElement, 'keydown', {cancelable: true, bubbles: true, keyCode: 13}); + browserTrigger(interactiveElement, 'keydown', {cancelable: true, bubbles: true, keyCode: 32}); + + expect(clickEvents).toEqual([elementType.toLowerCase() + '(false)', elementType.toLowerCase() + '(false)']); + } + ); + it('should trigger a click in browsers that provide `event.which` instead of `event.keyCode`', function() { compileElement( From 190bc1d52b046f6a9654451f5d1dbd6f0a925b29 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Mon, 3 Sep 2018 09:32:08 +0200 Subject: [PATCH 2/3] fixup! fix(ngAria.ngClick): restrict preventDefault on space / enter to non-interactive elements --- test/ngAria/ariaSpec.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/ngAria/ariaSpec.js b/test/ngAria/ariaSpec.js index 36f785e82edd..fb20f610b114 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -959,9 +959,7 @@ describe('$aria', function() { function createHTML(type) { var html = '<' + type + '>'; - if (type === 'INPUT' || 'TYPE' === 'A') { - return html; - } + if (type === 'INPUT' || 'TYPE' === 'A') return html; return html + ''; } @@ -975,10 +973,15 @@ describe('$aria', function() { var interactiveElement = element.find(elementType); // Use browserTrigger because it supports event bubbling + // 13 Enter browserTrigger(interactiveElement, 'keydown', {cancelable: true, bubbles: true, keyCode: 13}); - browserTrigger(interactiveElement, 'keydown', {cancelable: true, bubbles: true, keyCode: 32}); + expect(clickEvents).toEqual([elementType.toLowerCase() + '(false)']); + + clickEvents = []; - expect(clickEvents).toEqual([elementType.toLowerCase() + '(false)', elementType.toLowerCase() + '(false)']); + // 32 Space + browserTrigger(interactiveElement, 'keydown', {cancelable: true, bubbles: true, keyCode: 32}); + expect(clickEvents).toEqual([elementType.toLowerCase() + '(false)']); } ); From 30ab202fa749e7c98adff9b596cc40ca84a35c80 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Wed, 5 Sep 2018 19:33:33 +0200 Subject: [PATCH 3/3] fixup! fix(ngAria.ngClick): restrict preventDefault on space / enter to non-interactive elements --- src/ngAria/aria.js | 4 ++-- test/ngAria/ariaSpec.js | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/ngAria/aria.js b/src/ngAria/aria.js index 82216bfb464c..cc7e481c5588 100644 --- a/src/ngAria/aria.js +++ b/src/ngAria/aria.js @@ -389,9 +389,9 @@ ngAriaModule.directive('ngShow', ['$aria', function($aria) { var keyCode = event.which || event.keyCode; if (keyCode === 13 || keyCode === 32) { - // Prevent the default browser behavior (e.g. scrolling when pressing spacebar) ... + // If the event is triggered on a non-interactive element ... if (nodeBlackList.indexOf(event.target.nodeName) === -1) { - // ... but only when the event is triggered by a non-interactive element + // ... 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 fb20f610b114..e36fb9cc2778 100644 --- a/test/ngAria/ariaSpec.js +++ b/test/ngAria/ariaSpec.js @@ -957,11 +957,7 @@ describe('$aria', function() { they('should not prevent default keyboard action if an interactive $type element' + 'is nested inside ng-click', nodeBlackList, function(elementType) { function createHTML(type) { - var html = '<' + type + '>'; - - if (type === 'INPUT' || 'TYPE' === 'A') return html; - - return html + ''; + return '<' + type + '>'; } compileElement(