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
This commit is contained in:
Paul Gschwendtner
2018-07-10 16:36:10 +02:00
committed by Jeremy Elbourn
parent d75fa7591a
commit a1aa9e7eb8
5 changed files with 56 additions and 30 deletions
@@ -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');
+5 -2
View File
@@ -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);
});
}
+10 -4
View File
@@ -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]')!;
+15 -14
View File
@@ -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.');
+3 -1
View File
@@ -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(() => {