fix(a11y): focus monitor not checking children if monitor is called m…

…ultiple times with different parameters

With the current logic, if an element is being monitored with `checkChildren` set to `true` and then we start monitoring it with `false`, the initial monitor call will stop checking the children, because we take the `checkChildren` value from the most-recent call. This is something that we discussed in #19135 (comment), but now we have a case where it can happen: setting a tooltip on a sort header. The sort header looks for child focus, whereas the tooltip doesn't, causing the sort focus indication to break.

These changes fix the issue by treating all focus as `checkChildren`, as soon as there's at least one call to `monitor` with `checkChildren` turned on.

Fixes #19218.

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

@@ -277,6 +277,23 @@ describe('FocusMonitor', () => {
        // After 2 ticks, the timeout has cleared the origin. Default is 'program'.
        expect(changeHandler).toHaveBeenCalledWith('program');
      }));
+
+  it('should check children if monitor was called with different checkChildren', fakeAsync(() => {
+    const parent = fixture.nativeElement.querySelector('.parent');
+
+    focusMonitor.monitor(parent, true);
+    focusMonitor.monitor(parent, false);
+
+    // Simulate focus via mouse.
+    dispatchMouseEvent(buttonElement, 'mousedown');
+    buttonElement.focus();
+    fixture.detectChanges();
+    flush();
+
+    expect(parent.classList).toContain('cdk-focused');
+    expect(parent.classList).toContain('cdk-mouse-focused');
+  }));
+
 });
 
 describe('FocusMonitor with "eventual" detection', () => {
@@ -569,7 +586,7 @@ describe('FocusMonitor observable stream', () => {
 
 
 @Component({
-  template: `<button>focus me!</button>`
+  template: `<div class="parent"><button>focus me!</button></div>`
 })
 class PlainButton {}
 

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

@@ -235,11 +235,17 @@ export class FocusMonitor implements OnDestroy {
     // the shadow root, rather than the `document`, because the browser won't emit focus events
     // to the `document`, if focus is moving within the same shadow root.
     const rootNode = (_getShadowRoot(nativeElement) as HTMLElement|null) || this._getDocument();
+    const cachedInfo = this._elementInfo.get(nativeElement);
 
     // Check if we're already monitoring this element.
-    if (this._elementInfo.has(nativeElement)) {
-      const cachedInfo = this._elementInfo.get(nativeElement)!;
-      cachedInfo.checkChildren = checkChildren;
+    if (cachedInfo) {
+      if (checkChildren) {
+        // TODO(COMP-318): this can be problematic, because it'll turn all non-checkChildren
+        // observers into ones that behave as if `checkChildren` was turned on. We need a more
+        // robust solution.
+        cachedInfo.checkChildren = true;
+      }
+
       return cachedInfo.subject.asObservable();
     }