fix(aria-describer): exception when attempting to describe a non-element node (#9392)

* Fixes an error that was being thrown when trying to describe a non-element node. This could happen if the consumer is attempting to describe an `ng-container` which corresponds to a comment node.
* Switches a few cases, that had hard-coded the `Node.ELEMENT_NODE` constant for to Universal compatibility, to take the constant from the `Document`.

Relates to #8724.
This commit is contained in:
Kristiyan Kostadinov
2018-01-23 16:41:33 +01:00
committed by Jeremy Elbourn
parent 51363618c6
commit 4c7a4f33e0
4 changed files with 14 additions and 13 deletions
+5
View File
@@ -107,6 +107,11 @@ describe('AriaDescriber', () => {
expectMessages(['My Message']);
expectMessage(component.element1, 'My Message');
});
it('should not throw when attempting to describe a non-element node', () => {
const node: any = document.createComment('Not an element node');
expect(() => ariaDescriber.describe(node, 'This looks like an element')).not.toThrow();
});
});
function getMessagesContainer() {
+2 -2
View File
@@ -60,7 +60,7 @@ export class AriaDescriber {
* message element.
*/
describe(hostElement: Element, message: string) {
if (!message.trim()) {
if (hostElement.nodeType !== this._document.ELEMENT_NODE || !message.trim()) {
return;
}
@@ -75,7 +75,7 @@ export class AriaDescriber {
/** Removes the host element's aria-describedby reference to the message element. */
removeDescription(hostElement: Element, message: string) {
if (!message.trim()) {
if (hostElement.nodeType !== this._document.ELEMENT_NODE || !message.trim()) {
return;
}
+2 -8
View File
@@ -21,12 +21,6 @@ import {take} from 'rxjs/operators/take';
import {InteractivityChecker} from './interactivity-checker';
import {DOCUMENT} from '@angular/common';
/**
* Node type of element nodes. Used instead of Node.ELEMENT_NODE
* which is unsupported in Universal.
* @docs-private
*/
const ELEMENT_NODE_TYPE = 1;
/**
* Class that allows for trapping focus within a DOM element.
@@ -229,7 +223,7 @@ export class FocusTrap {
let children = root.children || root.childNodes;
for (let i = 0; i < children.length; i++) {
let tabbableChild = children[i].nodeType === ELEMENT_NODE_TYPE ?
let tabbableChild = children[i].nodeType === this._document.ELEMENT_NODE ?
this._getFirstTabbableElement(children[i] as HTMLElement) :
null;
@@ -251,7 +245,7 @@ export class FocusTrap {
let children = root.children || root.childNodes;
for (let i = children.length - 1; i >= 0; i--) {
let tabbableChild = children[i].nodeType === ELEMENT_NODE_TYPE ?
let tabbableChild = children[i].nodeType === this._document.ELEMENT_NODE ?
this._getLastTabbableElement(children[i] as HTMLElement) :
null;
+5 -3
View File
@@ -78,6 +78,8 @@ class SvgIconConfig {
*/
@Injectable()
export class MatIconRegistry {
private _document: Document;
/**
* URLs and cached SVG elements for individual icons. Keys are of the format "[namespace]:[icon]".
*/
@@ -108,8 +110,9 @@ export class MatIconRegistry {
constructor(
@Optional() private _httpClient: HttpClient,
private _sanitizer: DomSanitizer,
@Optional() @Inject(DOCUMENT) private _document?: any) {
@Optional() @Inject(DOCUMENT) document?: any) {
// TODO(crisbeto): make _document required next major release.
this._document = document;
}
/**
@@ -438,8 +441,7 @@ export class MatIconRegistry {
let svg = this._svgElementFromString('<svg></svg>');
for (let i = 0; i < element.childNodes.length; i++) {
// Note: 1 corresponds to `Node.ELEMENT_NODE` which we can't use in Universal.
if (element.childNodes[i].nodeType === 1) {
if (element.childNodes[i].nodeType === this._document.ELEMENT_NODE) {
svg.appendChild(element.childNodes[i].cloneNode(true));
}
}