Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
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
5 changes: 5 additions & 0 deletions src/cdk/a11y/aria-describer.spec.ts
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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