Skip to content

Commit 88f5ea4

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 3352201 commit 88f5ea4

File tree

5 files changed

+52
-28
lines changed

5 files changed

+52
-28
lines changed

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

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
patchElementFocus,
77
} from '@angular/cdk/testing';
88
import {Component} from '@angular/core';
9-
import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
9+
import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
1010
import {By} from '@angular/platform-browser';
1111
import {FocusMonitor, FocusOrigin, TOUCH_BUFFER_MS} from './focus-monitor';
1212
import {A11yModule} from '../index';
@@ -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');
@@ -213,6 +213,20 @@ describe('FocusMonitor', () => {
213213
expect(buttonElement.classList.length).toBe(0, 'button should not have any focus classes');
214214
}));
215215

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 a 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+
}));
216230
});
217231

218232

@@ -250,7 +264,7 @@ describe('cdkMonitorFocus', () => {
250264
dispatchKeyboardEvent(document, 'keydown', TAB);
251265
buttonElement.focus();
252266
fixture.detectChanges();
253-
tick();
267+
flush();
254268

255269
expect(buttonElement.classList.length)
256270
.toBe(2, 'button should have exactly 2 focus classes');
@@ -266,7 +280,7 @@ describe('cdkMonitorFocus', () => {
266280
dispatchMouseEvent(buttonElement, 'mousedown');
267281
buttonElement.focus();
268282
fixture.detectChanges();
269-
tick();
283+
flush();
270284

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

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ export class FocusMonitor implements OnDestroy {
189189
};
190190

191191
// When the touchstart event fires the focus event is not yet in the event queue. This means
192-
// we can't rely on the trick used above (setting timeout of 0ms). Instead we wait 650ms to
192+
// we can't rely on the trick used above (setting timeout of 1ms). Instead we wait 650ms to
193193
// see if a focus happens.
194194
let documentTouchstartListener = (event: TouchEvent) => {
195195
if (this._touchTimeoutId != null) {
@@ -261,7 +261,7 @@ export class FocusMonitor implements OnDestroy {
261261
*/
262262
private _setOriginForCurrentEventQueue(origin: FocusOrigin): void {
263263
this._origin = origin;
264-
this._originTimeoutId = setTimeout(() => this._origin = null, 0);
264+
this._originTimeoutId = setTimeout(() => this._origin = null, 1);
265265
}
266266

267267
/**

src/lib/menu/menu.spec.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import {async, ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
1+
import {
2+
async,
3+
ComponentFixture,
4+
fakeAsync,
5+
flush,
6+
inject,
7+
TestBed,
8+
tick,
9+
} from '@angular/core/testing';
210
import {By} from '@angular/platform-browser';
311
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
412
import {
@@ -363,7 +371,7 @@ describe('MatMenu', () => {
363371
fixture.detectChanges();
364372
tick(500);
365373
zone!.simulateZoneExit();
366-
tick();
374+
flush();
367375

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

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

@@ -116,7 +117,7 @@ describe('MatDrawer', () => {
116117

117118
drawer.componentInstance.open();
118119
fixture.detectChanges();
119-
tick();
120+
flush();
120121
fixture.detectChanges();
121122

122123
drawer.componentInstance.close().then(result => {
@@ -125,7 +126,7 @@ describe('MatDrawer', () => {
125126
expect(result.animationFinished).toBe(true);
126127
});
127128
fixture.detectChanges();
128-
tick();
129+
flush();
129130
fixture.detectChanges();
130131
}));
131132

@@ -144,7 +145,7 @@ describe('MatDrawer', () => {
144145
fixture.debugElement.query(By.css('.close')).nativeElement.click();
145146
fixture.detectChanges();
146147

147-
tick();
148+
flush();
148149
fixture.detectChanges();
149150

150151
expect(testComponent.openCount).toBe(1);
@@ -168,23 +169,23 @@ describe('MatDrawer', () => {
168169

169170
openButtonElement.click();
170171
fixture.detectChanges();
171-
tick();
172+
flush();
172173

173174
expect(testComponent.backdropClickedCount).toBe(0);
174175

175176
fixture.debugElement.query(By.css('.mat-drawer-backdrop')).nativeElement.click();
176177
fixture.detectChanges();
177-
tick();
178+
flush();
178179

179180
expect(testComponent.backdropClickedCount).toBe(1);
180181

181182
openButtonElement.click();
182183
fixture.detectChanges();
183-
tick();
184+
flush();
184185

185186
fixture.debugElement.query(By.css('.close')).nativeElement.click();
186187
fixture.detectChanges();
187-
tick();
188+
flush();
188189

189190
expect(testComponent.backdropClickedCount).toBe(1);
190191
}));
@@ -208,7 +209,7 @@ describe('MatDrawer', () => {
208209

209210
dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
210211
fixture.detectChanges();
211-
tick();
212+
flush();
212213

213214
expect(testComponent.closeCount).toBe(1, 'Expected one close event.');
214215
expect(testComponent.closeStartCount).toBe(1, 'Expected one close start event.');
@@ -271,12 +272,12 @@ describe('MatDrawer', () => {
271272
openButton.focus();
272273
drawer.open();
273274
fixture.detectChanges();
274-
tick();
275+
flush();
275276
drawerButton.focus();
276277

277278
drawer.close();
278279
fixture.detectChanges();
279-
tick();
280+
flush();
280281

281282
expect(document.activeElement)
282283
.toBe(openButton, 'Expected focus to be restored to the open button on close.');
@@ -508,7 +509,7 @@ describe('MatDrawerContainer', () => {
508509

509510
testComponent.drawerContainer.close();
510511
fixture.detectChanges();
511-
tick();
512+
flush();
512513

513514
expect(drawers.every(drawer => drawer.componentInstance.opened)).toBe(false);
514515
}));

src/lib/tooltip/tooltip.spec.ts

Lines changed: 3 additions & 2 deletions
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,
@@ -564,10 +565,10 @@ describe('MatTooltip', () => {
564565
fixture.detectChanges();
565566
}).not.toThrow();
566567

567-
tick(0);
568+
flush();
568569
}));
569570

570-
it('should not show the tooltip on progammatic focus', fakeAsync(() => {
571+
it('should not show the tooltip on programmatic focus', fakeAsync(() => {
571572
assertTooltipInstance(tooltipDirective, false);
572573

573574
buttonElement.focus();

0 commit comments

Comments
 (0)