Skip to content

Commit b75c7e5

Browse files
committed
fix(focus-origin): focus origin sometimes invalid in firefox 57
* 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
1 parent aa09628 commit b75c7e5

File tree

5 files changed

+55
-28
lines changed

5 files changed

+55
-28
lines changed

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

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ describe('FocusMonitor', () => {
5454
dispatchKeyboardEvent(document, 'keydown', TAB);
5555
buttonElement.focus();
5656
fixture.detectChanges();
57-
tick();
57+
flush();
5858

5959
expect(buttonElement.classList.length)
6060
.toBe(2, 'button should have exactly 2 focus classes');
@@ -70,7 +70,7 @@ describe('FocusMonitor', () => {
7070
dispatchMouseEvent(buttonElement, 'mousedown');
7171
buttonElement.focus();
7272
fixture.detectChanges();
73-
tick();
73+
flush();
7474

7575
expect(buttonElement.classList.length)
7676
.toBe(2, 'button should have exactly 2 focus classes');
@@ -114,7 +114,7 @@ describe('FocusMonitor', () => {
114114

115115
it('focusVia keyboard should simulate keyboard focus', fakeAsync(() => {
116116
focusMonitor.focusVia(buttonElement, 'keyboard');
117-
tick();
117+
flush();
118118

119119
expect(buttonElement.classList.length)
120120
.toBe(2, 'button should have exactly 2 focus classes');
@@ -128,7 +128,7 @@ describe('FocusMonitor', () => {
128128
it('focusVia mouse should simulate mouse focus', fakeAsync(() => {
129129
focusMonitor.focusVia(buttonElement, 'mouse');
130130
fixture.detectChanges();
131-
tick();
131+
flush();
132132

133133
expect(buttonElement.classList.length)
134134
.toBe(2, 'button should have exactly 2 focus classes');
@@ -142,7 +142,7 @@ describe('FocusMonitor', () => {
142142
it('focusVia mouse should simulate mouse focus', fakeAsync(() => {
143143
focusMonitor.focusVia(buttonElement, 'touch');
144144
fixture.detectChanges();
145-
tick();
145+
flush();
146146

147147
expect(buttonElement.classList.length)
148148
.toBe(2, 'button should have exactly 2 focus classes');
@@ -156,7 +156,7 @@ describe('FocusMonitor', () => {
156156
it('focusVia program should simulate programmatic focus', fakeAsync(() => {
157157
focusMonitor.focusVia(buttonElement, 'program');
158158
fixture.detectChanges();
159-
tick();
159+
flush();
160160

161161
expect(buttonElement.classList.length)
162162
.toBe(2, 'button should have exactly 2 focus classes');
@@ -212,6 +212,21 @@ describe('FocusMonitor', () => {
212212

213213
expect(buttonElement.classList.length).toBe(0, 'button should not have any focus classes');
214214
}));
215+
216+
it('should not clear the focus origin too early in the current event loop', fakeAsync(() => {
217+
dispatchKeyboardEvent(document, 'keydown', TAB);
218+
219+
// Simulate the behavior of Firefox 57 where the focus event sometimes happens *one* tick later.
220+
tick();
221+
222+
buttonElement.focus();
223+
224+
// Since the timeout doesn't clear the focus origin too early as with the `0ms` timeout, the
225+
// focus origin should be reported properly.
226+
expect(changeHandler).toHaveBeenCalledWith('keyboard');
227+
228+
flush();
229+
}));
215230
});
216231

217232

@@ -250,7 +265,7 @@ describe('cdkMonitorFocus', () => {
250265
dispatchKeyboardEvent(document, 'keydown', TAB);
251266
buttonElement.focus();
252267
fixture.detectChanges();
253-
tick();
268+
flush();
254269

255270
expect(buttonElement.classList.length)
256271
.toBe(2, 'button should have exactly 2 focus classes');
@@ -266,7 +281,7 @@ describe('cdkMonitorFocus', () => {
266281
dispatchMouseEvent(buttonElement, 'mousedown');
267282
buttonElement.focus();
268283
fixture.detectChanges();
269-
tick();
284+
flush();
270285

271286
expect(buttonElement.classList.length)
272287
.toBe(2, 'button should have exactly 2 focus classes');

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ export class FocusMonitor implements OnDestroy {
173173
};
174174

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

src/lib/menu/menu.spec.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
1+
import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
22
import {By} from '@angular/platform-browser';
33
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
44
import {
@@ -176,7 +176,9 @@ describe('MatMenu', () => {
176176
dispatchFakeEvent(triggerEl, 'mousedown');
177177
triggerEl.click();
178178
fixture.detectChanges();
179-
tick();
179+
180+
// Flush due to the additional tick that is necessary for the FocusMonitor.
181+
flush();
180182

181183
expect(overlayContainerElement.querySelector('.mat-menu-panel')!.scrollTop).toBe(0);
182184
}));
@@ -398,7 +400,9 @@ describe('MatMenu', () => {
398400

399401
dispatchKeyboardEvent(panel, 'keydown', DOWN_ARROW);
400402
fixture.detectChanges();
401-
tick();
403+
404+
// Flush due to the additional tick that is necessary for the FocusMonitor.
405+
flush();
402406

403407
// We skip to the third item, because the second one is disabled.
404408
expect(items[2].classList).toContain('cdk-focused');
@@ -495,7 +499,9 @@ describe('MatMenu', () => {
495499
fixture.detectChanges();
496500
tick(500);
497501
zone!.simulateZoneExit();
498-
tick();
502+
503+
// Flush due to the additional tick that is necessary for the FocusMonitor.
504+
flush();
499505

500506
const item = document.querySelector('.mat-menu-panel [mat-menu-item]')!;
501507

src/lib/sidenav/drawer.spec.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
ComponentFixture,
66
TestBed,
77
discardPeriodicTasks,
8+
flush,
89
} from '@angular/core/testing';
910
import {Component, ElementRef, ViewChild} from '@angular/core';
1011
import {By} from '@angular/platform-browser';
@@ -75,7 +76,7 @@ describe('MatDrawer', () => {
7576
drawerBackdropElement.nativeElement.style.transition = 'none';
7677
fixture.debugElement.query(By.css('.open')).nativeElement.click();
7778
fixture.detectChanges();
78-
tick();
79+
flush();
7980
fixture.detectChanges();
8081

8182
fixture.debugElement.query(By.css('.close')).nativeElement.click();
@@ -84,7 +85,7 @@ describe('MatDrawer', () => {
8485
expect(testComponent.closeCount).toBe(0);
8586
expect(testComponent.closeStartCount).toBe(0);
8687

87-
tick();
88+
flush();
8889
expect(testComponent.closeStartCount).toBe(1);
8990
fixture.detectChanges();
9091

@@ -112,12 +113,12 @@ describe('MatDrawer', () => {
112113

113114
drawer.componentInstance.open();
114115
fixture.detectChanges();
115-
tick();
116+
flush();
116117
fixture.detectChanges();
117118

118119
drawer.componentInstance.close().then(result => expect(result).toBe('close'));
119120
fixture.detectChanges();
120-
tick();
121+
flush();
121122
fixture.detectChanges();
122123
}));
123124

@@ -136,7 +137,7 @@ describe('MatDrawer', () => {
136137
fixture.debugElement.query(By.css('.close')).nativeElement.click();
137138
fixture.detectChanges();
138139

139-
tick();
140+
flush();
140141
fixture.detectChanges();
141142

142143
expect(testComponent.openCount).toBe(1);
@@ -160,23 +161,23 @@ describe('MatDrawer', () => {
160161

161162
openButtonElement.click();
162163
fixture.detectChanges();
163-
tick();
164+
flush();
164165

165166
expect(testComponent.backdropClickedCount).toBe(0);
166167

167168
fixture.debugElement.query(By.css('.mat-drawer-backdrop')).nativeElement.click();
168169
fixture.detectChanges();
169-
tick();
170+
flush();
170171

171172
expect(testComponent.backdropClickedCount).toBe(1);
172173

173174
openButtonElement.click();
174175
fixture.detectChanges();
175-
tick();
176+
flush();
176177

177178
fixture.debugElement.query(By.css('.close')).nativeElement.click();
178179
fixture.detectChanges();
179-
tick();
180+
flush();
180181

181182
expect(testComponent.backdropClickedCount).toBe(1);
182183
}));
@@ -200,7 +201,7 @@ describe('MatDrawer', () => {
200201

201202
dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
202203
fixture.detectChanges();
203-
tick();
204+
flush();
204205

205206
expect(testComponent.closeCount).toBe(1, 'Expected one close event.');
206207
expect(testComponent.closeStartCount).toBe(1, 'Expected one close start event.');
@@ -263,12 +264,12 @@ describe('MatDrawer', () => {
263264
openButton.focus();
264265
drawer.open();
265266
fixture.detectChanges();
266-
tick();
267+
flush();
267268
drawerButton.focus();
268269

269270
drawer.close();
270271
fixture.detectChanges();
271-
tick();
272+
flush();
272273

273274
expect(document.activeElement)
274275
.toBe(openButton, 'Expected focus to be restored to the open button on close.');
@@ -521,7 +522,7 @@ describe('MatDrawerContainer', () => {
521522

522523
testComponent.drawerContainer.close();
523524
fixture.detectChanges();
524-
tick();
525+
flush();
525526

526527
expect(drawers.every(drawer => drawer.componentInstance.opened)).toBe(false);
527528
}));

src/lib/tooltip/tooltip.spec.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
async,
33
ComponentFixture,
44
fakeAsync,
5+
flush,
56
flushMicrotasks,
67
inject,
78
TestBed,
@@ -575,7 +576,8 @@ describe('MatTooltip', () => {
575576
fixture.detectChanges();
576577
}).not.toThrow();
577578

578-
tick(0);
579+
// Flush due to the additional tick that is necessary for the FocusMonitor.
580+
flush();
579581
}));
580582

581583
it('should not show the tooltip on progammatic focus', fakeAsync(() => {

0 commit comments

Comments
 (0)