Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(aria/ngClick): check if element is contenteditable before block…
Browse files Browse the repository at this point in the history 
…ing spacebar

`ngAria`'s `ngClick` blocks spacebar keypresses on non-blacklisted
elements, which is an issue when the element is `contenteditable`.

Closes #16762
  • Loading branch information
@Eblax @gkalpak
Eblax authored and gkalpak committed Dec 2, 2018
1 parent 3b333f3 commit 289374a
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/ngAria/aria.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
129 changes: 109 additions & 20 deletions test/ngAria/ariaSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
'<section>' +
'<div ng-click="onClick($event)"></div>' +
'<ul><li ng-click="onClick($event)"></li></ul>' +
'</section>');

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(
'<section>' +
// No attribute.
'<div id="no-attribute">' +
'<div ng-click="onClick($event)"></div>' +
'<ul ng-click="onClick($event)"><li></li></ul>' +
'</div>' +

// Value: ""
'<div id="value-empty">' +
'<div ng-click="onClick($event)" contenteditable=""></div>' +
'<ul ng-click="onClick($event)"><li contenteditable=""></li></ul>' +
'</div>' +

// Value: "true"
'<div id="value-true">' +
'<div ng-click="onClick($event)" contenteditable="true"></div>' +
'<ul ng-click="onClick($event)"><li contenteditable="true"></li></ul>' +
'</div>' +

// Value: "false"
'<div id="value-false">' +
'<div ng-click="onClick($event)" contenteditable="false"></div>' +
'<ul ng-click="onClick($event)"><li contenteditable="false"></li></ul>' +
'</div>' +
'</section>');

// 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) {
Expand Down Expand Up @@ -981,26 +1090,6 @@ describe('$aria', function() {
}
);

it('should trigger a click in browsers that provide `event.which` instead of `event.keyCode`',
function() {
compileElement(
'<section>' +
'<div ng-click="onClick($event)"></div>' +
'<ul><li ng-click="onClick($event)"></li></ul>' +
'</section>');

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');
Expand Down

0 comments on commit 289374a

Please sign in to comment.