fix(focus-origin): focus origin sometimes invalid in firefox 57 (#8669)

Sometimes the focus origin is not valid because Firefox seems to focus a tick after the keyboard/interaction event fired a tick before. To ensure the focus origin is always correct, the focus origin will be added to the next tick event loop

Fixes #6984

Author: devversion

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

@@ -54,7 +54,7 @@ describe('FocusMonitor', () => {
     dispatchKeyboardEvent(document, 'keydown', TAB);
     buttonElement.focus();
     fixture.detectChanges();
-    tick();
+    flush();
 
     expect(buttonElement.classList.length)
         .toBe(2, 'button should have exactly 2 focus classes');
@@ -70,7 +70,7 @@ describe('FocusMonitor', () => {
     dispatchMouseEvent(buttonElement, 'mousedown');
     buttonElement.focus();
     fixture.detectChanges();
-    tick();
+    flush();
 
     expect(buttonElement.classList.length)
         .toBe(2, 'button should have exactly 2 focus classes');
@@ -114,7 +114,7 @@ describe('FocusMonitor', () => {
 
   it('focusVia keyboard should simulate keyboard focus', fakeAsync(() => {
     focusMonitor.focusVia(buttonElement, 'keyboard');
-    tick();
+    flush();
 
     expect(buttonElement.classList.length)
         .toBe(2, 'button should have exactly 2 focus classes');
@@ -128,7 +128,7 @@ describe('FocusMonitor', () => {
   it('focusVia mouse should simulate mouse focus', fakeAsync(() => {
     focusMonitor.focusVia(buttonElement, 'mouse');
     fixture.detectChanges();
-    tick();
+    flush();
 
     expect(buttonElement.classList.length)
         .toBe(2, 'button should have exactly 2 focus classes');
@@ -142,7 +142,7 @@ describe('FocusMonitor', () => {
   it('focusVia mouse should simulate mouse focus', fakeAsync(() => {
     focusMonitor.focusVia(buttonElement, 'touch');
     fixture.detectChanges();
-    tick();
+    flush();
 
     expect(buttonElement.classList.length)
         .toBe(2, 'button should have exactly 2 focus classes');
@@ -156,7 +156,7 @@ describe('FocusMonitor', () => {
   it('focusVia program should simulate programmatic focus', fakeAsync(() => {
     focusMonitor.focusVia(buttonElement, 'program');
     fixture.detectChanges();
-    tick();
+    flush();
 
     expect(buttonElement.classList.length)
         .toBe(2, 'button should have exactly 2 focus classes');
@@ -218,13 +218,27 @@ describe('FocusMonitor', () => {
 
     focusMonitor.focusVia(buttonElement, 'program', {preventScroll: true});
     fixture.detectChanges();
-    tick();
+    flush();
 
     expect(buttonElement.focus).toHaveBeenCalledWith(jasmine.objectContaining({
       preventScroll: true
     }));
   }));
 
+  it('should not clear the focus origin too early in the current event loop', fakeAsync(() => {
+    dispatchKeyboardEvent(document, 'keydown', TAB);
+
+    // Simulate the behavior of Firefox 57 where the focus event sometimes happens *one* tick later.
+    tick();
+
+    buttonElement.focus();
+
+    // Since the timeout doesn't clear the focus origin too early as with the `0ms` timeout, the
+    // focus origin should be reported properly.
+    expect(changeHandler).toHaveBeenCalledWith('keyboard');
+
+    flush();
+  }));
 });
 
 
@@ -263,7 +277,7 @@ describe('cdkMonitorFocus', () => {
       dispatchKeyboardEvent(document, 'keydown', TAB);
       buttonElement.focus();
       fixture.detectChanges();
-      tick();
+      flush();
 
       expect(buttonElement.classList.length)
           .toBe(2, 'button should have exactly 2 focus classes');
@@ -279,7 +293,7 @@ describe('cdkMonitorFocus', () => {
       dispatchMouseEvent(buttonElement, 'mousedown');
       buttonElement.focus();
       fixture.detectChanges();
-      tick();
+      flush();
 
       expect(buttonElement.classList.length)
           .toBe(2, 'button should have exactly 2 focus classes');

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

@@ -185,7 +185,7 @@ export class FocusMonitor implements OnDestroy {
     };
 
     // When the touchstart event fires the focus event is not yet in the event queue. This means
-    // we can't rely on the trick used above (setting timeout of 0ms). Instead we wait 650ms to
+    // we can't rely on the trick used above (setting timeout of 1ms). Instead we wait 650ms to
     // see if a focus happens.
     let documentTouchstartListener = (event: TouchEvent) => {
       if (this._touchTimeoutId != null) {
@@ -258,7 +258,10 @@ export class FocusMonitor implements OnDestroy {
   private _setOriginForCurrentEventQueue(origin: FocusOrigin): void {
     this._ngZone.runOutsideAngular(() => {
       this._origin = origin;
-      this._originTimeoutId = setTimeout(() => this._origin = null);
+      // Sometimes the focus origin won't be valid in Firefox because Firefox seems to focus *one*
+      // tick after the interaction event fired. To ensure the focus origin is always correct,
+      // the focus origin will be determined at the beginning of the next tick.
+      this._originTimeoutId = setTimeout(() => this._origin = null, 1);
     });
   }
 

src/lib/menu/menu.spec.ts

@@ -1,4 +1,4 @@
-import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
+import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 import {NoopAnimationsModule} from '@angular/platform-browser/animations';
 import {
@@ -176,7 +176,9 @@ describe('MatMenu', () => {
     dispatchFakeEvent(triggerEl, 'mousedown');
     triggerEl.click();
     fixture.detectChanges();
-    tick();
+
+    // Flush due to the additional tick that is necessary for the FocusMonitor.
+    flush();
 
     expect(overlayContainerElement.querySelector('.mat-menu-panel')!.scrollTop).toBe(0);
   }));
@@ -398,7 +400,9 @@ describe('MatMenu', () => {
 
       dispatchKeyboardEvent(panel, 'keydown', DOWN_ARROW);
       fixture.detectChanges();
-      tick();
+
+      // Flush due to the additional tick that is necessary for the FocusMonitor.
+      flush();
 
       // We skip to the third item, because the second one is disabled.
       expect(items[2].classList).toContain('cdk-focused');
@@ -495,7 +499,9 @@ describe('MatMenu', () => {
       fixture.detectChanges();
       tick(500);
       zone!.simulateZoneExit();
-      tick();
+
+      // Flush due to the additional tick that is necessary for the FocusMonitor.
+      flush();
 
       const item = document.querySelector('.mat-menu-panel [mat-menu-item]')!;
 

src/lib/sidenav/drawer.spec.ts

@@ -5,6 +5,7 @@ import {
   ComponentFixture,
   TestBed,
   discardPeriodicTasks,
+  flush,
 } from '@angular/core/testing';
 import {Component, ElementRef, ViewChild} from '@angular/core';
 import {By} from '@angular/platform-browser';
@@ -75,7 +76,7 @@ describe('MatDrawer', () => {
       drawerBackdropElement.nativeElement.style.transition = 'none';
       fixture.debugElement.query(By.css('.open')).nativeElement.click();
       fixture.detectChanges();
-      tick();
+      flush();
       fixture.detectChanges();
 
       fixture.debugElement.query(By.css('.close')).nativeElement.click();
@@ -84,7 +85,7 @@ describe('MatDrawer', () => {
       expect(testComponent.closeCount).toBe(0);
       expect(testComponent.closeStartCount).toBe(0);
 
-      tick();
+      flush();
       expect(testComponent.closeStartCount).toBe(1);
       fixture.detectChanges();
 
@@ -112,12 +113,12 @@ describe('MatDrawer', () => {
 
       drawer.componentInstance.open();
       fixture.detectChanges();
-      tick();
+      flush();
       fixture.detectChanges();
 
       drawer.componentInstance.close().then(result => expect(result).toBe('close'));
       fixture.detectChanges();
-      tick();
+      flush();
       fixture.detectChanges();
     }));
 
@@ -136,7 +137,7 @@ describe('MatDrawer', () => {
       fixture.debugElement.query(By.css('.close')).nativeElement.click();
       fixture.detectChanges();
 
-      tick();
+      flush();
       fixture.detectChanges();
 
       expect(testComponent.openCount).toBe(1);
@@ -160,23 +161,23 @@ describe('MatDrawer', () => {
 
       openButtonElement.click();
       fixture.detectChanges();
-      tick();
+      flush();
 
       expect(testComponent.backdropClickedCount).toBe(0);
 
       fixture.debugElement.query(By.css('.mat-drawer-backdrop')).nativeElement.click();
       fixture.detectChanges();
-      tick();
+      flush();
 
       expect(testComponent.backdropClickedCount).toBe(1);
 
       openButtonElement.click();
       fixture.detectChanges();
-      tick();
+      flush();
 
       fixture.debugElement.query(By.css('.close')).nativeElement.click();
       fixture.detectChanges();
-      tick();
+      flush();
 
       expect(testComponent.backdropClickedCount).toBe(1);
     }));
@@ -200,7 +201,7 @@ describe('MatDrawer', () => {
 
       dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
       fixture.detectChanges();
-      tick();
+      flush();
 
       expect(testComponent.closeCount).toBe(1, 'Expected one close event.');
       expect(testComponent.closeStartCount).toBe(1, 'Expected one close start event.');
@@ -263,12 +264,12 @@ describe('MatDrawer', () => {
       openButton.focus();
       drawer.open();
       fixture.detectChanges();
-      tick();
+      flush();
       drawerButton.focus();
 
       drawer.close();
       fixture.detectChanges();
-      tick();
+      flush();
 
       expect(document.activeElement)
           .toBe(openButton, 'Expected focus to be restored to the open button on close.');
@@ -521,7 +522,7 @@ describe('MatDrawerContainer', () => {
 
     testComponent.drawerContainer.close();
     fixture.detectChanges();
-    tick();
+    flush();
 
     expect(drawers.every(drawer => drawer.componentInstance.opened)).toBe(false);
   }));
@@ -669,7 +670,7 @@ describe('MatDrawerContainer', () => {
       // Close the drawer and resolve the close animation.
       fixture.componentInstance.drawer.close();
       fixture.detectChanges();
-      tick();
+      flush();
       fixture.detectChanges();
 
       expect(content.style.marginLeft).toBe('', 'Margin should be removed after drawer close.');

src/lib/tooltip/tooltip.spec.ts

@@ -2,6 +2,7 @@ import {
   async,
   ComponentFixture,
   fakeAsync,
+  flush,
   flushMicrotasks,
   inject,
   TestBed,
@@ -575,7 +576,8 @@ describe('MatTooltip', () => {
         fixture.detectChanges();
       }).not.toThrow();
 
-      tick(0);
+      // Flush due to the additional tick that is necessary for the FocusMonitor.
+      flush();
     }));
 
     it('should not show the tooltip on progammatic focus', fakeAsync(() => {