Skip to content

Commit 92c29c8

Browse files
crisbetommalerba
authored andcommitted
fix(cdk/a11y): error if FocusMonitor is used on non-element nodes (#20640)
* Fixes an error thrown when trying to stop monitoring a non-element node. Now `FocusMonitor.monitor` is a noop. * Fixes an error on init if the `cdkMonitorFocus` directive is placed on a non-element node. Related to #20632. (cherry picked from commit 9edab81)
1 parent 9a0c4c0 commit 92c29c8

File tree

2 files changed

+24
-7
lines changed

2 files changed

+24
-7
lines changed

src/cdk/a11y/focus-monitor/focus-monitor.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ describe('cdkMonitorFocus', () => {
351351
ComplexComponentWithMonitorElementFocus,
352352
ComplexComponentWithMonitorSubtreeFocus,
353353
ComplexComponentWithMonitorSubtreeFocusAndMonitorElementFocus,
354+
FocusMonitorOnCommentNode,
354355
],
355356
}).compileComponents();
356357
});
@@ -548,6 +549,14 @@ describe('cdkMonitorFocus', () => {
548549
expect(childElement.classList).toContain('cdk-keyboard-focused');
549550
}));
550551
});
552+
553+
it('should not throw when trying to monitor focus on a non-element node', () => {
554+
expect(() => {
555+
const fixture = TestBed.createComponent(FocusMonitorOnCommentNode);
556+
fixture.detectChanges();
557+
fixture.destroy();
558+
}).not.toThrow();
559+
});
551560
});
552561

553562
describe('FocusMonitor observable stream', () => {
@@ -610,7 +619,14 @@ class ComplexComponentWithMonitorElementFocus {}
610619
})
611620
class ComplexComponentWithMonitorSubtreeFocus {}
612621

622+
613623
@Component({
614624
template: `<div cdkMonitorSubtreeFocus><button cdkMonitorElementFocus></button></div>`
615625
})
616626
class ComplexComponentWithMonitorSubtreeFocusAndMonitorElementFocus {}
627+
628+
629+
@Component({
630+
template: `<ng-container cdkMonitorElementFocus></ng-container>`
631+
})
632+
class FocusMonitorOnCommentNode {}

src/cdk/a11y/focus-monitor/focus-monitor.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,13 @@ export class FocusMonitor implements OnDestroy {
225225

226226
monitor(element: HTMLElement | ElementRef<HTMLElement>,
227227
checkChildren: boolean = false): Observable<FocusOrigin> {
228-
// Do nothing if we're not on the browser platform.
229-
if (!this._platform.isBrowser) {
228+
const nativeElement = coerceElement(element);
229+
230+
// Do nothing if we're not on the browser platform or the passed in node isn't an element.
231+
if (!this._platform.isBrowser || nativeElement.nodeType !== 1) {
230232
return observableOf(null);
231233
}
232234

233-
const nativeElement = coerceElement(element);
234-
235235
// If the element is inside the shadow DOM, we need to bind our focus/blur listeners to
236236
// the shadow root, rather than the `document`, because the browser won't emit focus events
237237
// to the `document`, if focus is moving within the same shadow root.
@@ -570,10 +570,11 @@ export class CdkMonitorFocus implements AfterViewInit, OnDestroy {
570570
constructor(private _elementRef: ElementRef<HTMLElement>, private _focusMonitor: FocusMonitor) {}
571571

572572
ngAfterViewInit() {
573+
const element = this._elementRef.nativeElement;
573574
this._monitorSubscription = this._focusMonitor.monitor(
574-
this._elementRef,
575-
this._elementRef.nativeElement.hasAttribute('cdkMonitorSubtreeFocus'))
576-
.subscribe(origin => this.cdkFocusChange.emit(origin));
575+
element,
576+
element.nodeType === 1 && element.hasAttribute('cdkMonitorSubtreeFocus'))
577+
.subscribe(origin => this.cdkFocusChange.emit(origin));
577578
}
578579

579580
ngOnDestroy() {

0 commit comments

Comments
 (0)