Skip to content

Commit 27b1bad

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 c3d7cd9 commit 27b1bad

File tree

4 files changed

+43
-26
lines changed

4 files changed

+43
-26
lines changed

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

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {TAB} from '@angular/cdk/keycodes';
22
import {dispatchFakeEvent, dispatchKeyboardEvent, dispatchMouseEvent} from '@angular/cdk/testing';
33
import {Component} from '@angular/core';
4-
import {ComponentFixture, fakeAsync, inject, TestBed, tick} from '@angular/core/testing';
4+
import {ComponentFixture, fakeAsync, flush, inject, TestBed, tick} from '@angular/core/testing';
55
import {By} from '@angular/platform-browser';
66
import {FocusMonitor, FocusOrigin, TOUCH_BUFFER_MS} from './focus-monitor';
77
import {A11yModule} from './index';
@@ -49,7 +49,7 @@ describe('FocusMonitor', () => {
4949
dispatchKeyboardEvent(document, 'keydown', TAB);
5050
buttonElement.focus();
5151
fixture.detectChanges();
52-
tick();
52+
flush();
5353

5454
expect(buttonElement.classList.length)
5555
.toBe(2, 'button should have exactly 2 focus classes');
@@ -65,7 +65,7 @@ describe('FocusMonitor', () => {
6565
dispatchMouseEvent(buttonElement, 'mousedown');
6666
buttonElement.focus();
6767
fixture.detectChanges();
68-
tick();
68+
flush();
6969

7070
expect(buttonElement.classList.length)
7171
.toBe(2, 'button should have exactly 2 focus classes');
@@ -109,7 +109,7 @@ describe('FocusMonitor', () => {
109109

110110
it('focusVia keyboard should simulate keyboard focus', fakeAsync(() => {
111111
focusMonitor.focusVia(buttonElement, 'keyboard');
112-
tick();
112+
flush();
113113

114114
expect(buttonElement.classList.length)
115115
.toBe(2, 'button should have exactly 2 focus classes');
@@ -123,7 +123,7 @@ describe('FocusMonitor', () => {
123123
it('focusVia mouse should simulate mouse focus', fakeAsync(() => {
124124
focusMonitor.focusVia(buttonElement, 'mouse');
125125
fixture.detectChanges();
126-
tick();
126+
flush();
127127

128128
expect(buttonElement.classList.length)
129129
.toBe(2, 'button should have exactly 2 focus classes');
@@ -137,7 +137,7 @@ describe('FocusMonitor', () => {
137137
it('focusVia mouse should simulate mouse focus', fakeAsync(() => {
138138
focusMonitor.focusVia(buttonElement, 'touch');
139139
fixture.detectChanges();
140-
tick();
140+
flush();
141141

142142
expect(buttonElement.classList.length)
143143
.toBe(2, 'button should have exactly 2 focus classes');
@@ -151,7 +151,7 @@ describe('FocusMonitor', () => {
151151
it('focusVia program should simulate programmatic focus', fakeAsync(() => {
152152
focusMonitor.focusVia(buttonElement, 'program');
153153
fixture.detectChanges();
154-
tick();
154+
flush();
155155

156156
expect(buttonElement.classList.length)
157157
.toBe(2, 'button should have exactly 2 focus classes');
@@ -194,6 +194,21 @@ describe('FocusMonitor', () => {
194194

195195
expect(buttonElement.classList.length).toBe(0, 'button should not have any focus classes');
196196
}));
197+
198+
it('should not clear the focus origin too early in the current event loop', fakeAsync(() => {
199+
dispatchKeyboardEvent(document, 'keydown', TAB);
200+
201+
// Simulate the behavior of Firefox 57 where the focus event sometimes happens a tick later.
202+
tick();
203+
204+
buttonElement.focus();
205+
206+
// Since the timeout doesn't clear the focus origin too early as with the `0ms` timeout, the
207+
// focus origin should be reported properly.
208+
expect(changeHandler).toHaveBeenCalledWith('keyboard');
209+
210+
flush();
211+
}));
197212
});
198213

199214

@@ -231,7 +246,7 @@ describe('cdkMonitorFocus', () => {
231246
dispatchKeyboardEvent(document, 'keydown', TAB);
232247
buttonElement.focus();
233248
fixture.detectChanges();
234-
tick();
249+
flush();
235250

236251
expect(buttonElement.classList.length)
237252
.toBe(2, 'button should have exactly 2 focus classes');
@@ -247,7 +262,7 @@ describe('cdkMonitorFocus', () => {
247262
dispatchMouseEvent(buttonElement, 'mousedown');
248263
buttonElement.focus();
249264
fixture.detectChanges();
250-
tick();
265+
flush();
251266

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

src/cdk/a11y/focus-monitor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ export class FocusMonitor {
179179
};
180180

181181
// When the touchstart event fires the focus event is not yet in the event queue. This means
182-
// we can't rely on the trick used above (setting timeout of 0ms). Instead we wait 650ms to
182+
// we can't rely on the trick used above (setting timeout of 1ms). Instead we wait 650ms to
183183
// see if a focus happens.
184184
let documentTouchstartListener = (event: TouchEvent) => {
185185
if (this._touchTimeout != null) {
@@ -246,7 +246,7 @@ export class FocusMonitor {
246246
*/
247247
private _setOriginForCurrentEventQueue(origin: FocusOrigin): void {
248248
this._origin = origin;
249-
setTimeout(() => this._origin = null, 0);
249+
setTimeout(() => this._origin = null, 1);
250250
}
251251

252252
/**

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';
@@ -74,7 +75,7 @@ describe('MatDrawer', () => {
7475
drawerBackdropElement.nativeElement.style.transition = 'none';
7576
fixture.debugElement.query(By.css('.open')).nativeElement.click();
7677
fixture.detectChanges();
77-
tick();
78+
flush();
7879
fixture.detectChanges();
7980

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

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

@@ -115,7 +116,7 @@ describe('MatDrawer', () => {
115116

116117
drawer.componentInstance.open();
117118
fixture.detectChanges();
118-
tick();
119+
flush();
119120
fixture.detectChanges();
120121

121122
drawer.componentInstance.close().then(result => {
@@ -124,7 +125,7 @@ describe('MatDrawer', () => {
124125
expect(result.animationFinished).toBe(true);
125126
});
126127
fixture.detectChanges();
127-
tick();
128+
flush();
128129
fixture.detectChanges();
129130
}));
130131

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

146-
tick();
147+
flush();
147148
fixture.detectChanges();
148149

149150
expect(testComponent.openCount).toBe(1);
@@ -167,23 +168,23 @@ describe('MatDrawer', () => {
167168

168169
openButtonElement.click();
169170
fixture.detectChanges();
170-
tick();
171+
flush();
171172

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

174175
fixture.debugElement.query(By.css('.mat-drawer-backdrop')).nativeElement.click();
175176
fixture.detectChanges();
176-
tick();
177+
flush();
177178

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

180181
openButtonElement.click();
181182
fixture.detectChanges();
182-
tick();
183+
flush();
183184

184185
fixture.debugElement.query(By.css('.close')).nativeElement.click();
185186
fixture.detectChanges();
186-
tick();
187+
flush();
187188

188189
expect(testComponent.backdropClickedCount).toBe(1);
189190
}));
@@ -207,7 +208,7 @@ describe('MatDrawer', () => {
207208

208209
dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
209210
fixture.detectChanges();
210-
tick();
211+
flush();
211212

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

276277
drawer.close();
277278
fixture.detectChanges();
278-
tick();
279+
flush();
279280

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

508509
testComponent.drawerContainer.close();
509510
fixture.detectChanges();
510-
tick();
511+
flush();
511512

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

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)