Skip to content

Commit

Permalink
fix(aria-describer): exception when attempting to describe a non-elem…
Browse files Browse the repository at this point in the history
…ent 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.
  • Loading branch information
crisbeto authored and jelbourn committed Jan 23, 2018
1 parent 5136361 commit 4c7a4f3
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 13 deletions.
5 changes: 5 additions & 0 deletions src/cdk/a11y/aria-describer.spec.ts
Expand Up @@ -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() {
Expand Down
4 changes: 2 additions & 2 deletions src/cdk/a11y/aria-describer.ts
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
10 changes: 2 additions & 8 deletions src/cdk/a11y/focus-trap.ts
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -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;

Expand Down
8 changes: 5 additions & 3 deletions src/lib/icon/icon-registry.ts
Expand Up @@ -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]".
*/
Expand Down Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -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));
}
}
Expand Down

0 comments on commit 4c7a4f3

Please sign in to comment.