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

fix(ngAria): check if element is contenteditable before blocking spac… #16762

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ngAria/aria.js
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
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