Skip to content

Commit 12b19d1

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 6e865b7 commit 12b19d1

File tree

4 files changed

+41
-25
lines changed

4 files changed

+41
-25
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: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {fakeAsync, async, tick, ComponentFixture, TestBed} from '@angular/core/testing';
1+
import {fakeAsync, async, tick, ComponentFixture, TestBed, flush} from '@angular/core/testing';
22
import {Component, ElementRef, ViewChild} from '@angular/core';
33
import {By} from '@angular/platform-browser';
44
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
@@ -66,7 +66,7 @@ describe('MatDrawer', () => {
6666
drawerBackdropElement.nativeElement.style.transition = 'none';
6767
fixture.debugElement.query(By.css('.open')).nativeElement.click();
6868
fixture.detectChanges();
69-
tick();
69+
flush();
7070
fixture.detectChanges();
7171

7272
fixture.debugElement.query(By.css('.close')).nativeElement.click();
@@ -75,7 +75,7 @@ describe('MatDrawer', () => {
7575
expect(testComponent.closeCount).toBe(0);
7676
expect(testComponent.closeStartCount).toBe(0);
7777

78-
tick();
78+
flush();
7979
expect(testComponent.closeStartCount).toBe(1);
8080
fixture.detectChanges();
8181

@@ -107,7 +107,7 @@ describe('MatDrawer', () => {
107107

108108
drawer.componentInstance.open();
109109
fixture.detectChanges();
110-
tick();
110+
flush();
111111
fixture.detectChanges();
112112

113113
drawer.componentInstance.close().then(result => {
@@ -116,7 +116,7 @@ describe('MatDrawer', () => {
116116
expect(result.animationFinished).toBe(true);
117117
});
118118
fixture.detectChanges();
119-
tick();
119+
flush();
120120
fixture.detectChanges();
121121
}));
122122

@@ -158,23 +158,23 @@ describe('MatDrawer', () => {
158158

159159
openButtonElement.click();
160160
fixture.detectChanges();
161-
tick();
161+
flush();
162162

163163
expect(testComponent.backdropClickedCount).toBe(0);
164164

165165
fixture.debugElement.query(By.css('.mat-drawer-backdrop')).nativeElement.click();
166166
fixture.detectChanges();
167-
tick();
167+
flush();
168168

169169
expect(testComponent.backdropClickedCount).toBe(1);
170170

171171
openButtonElement.click();
172172
fixture.detectChanges();
173-
tick();
173+
flush();
174174

175175
fixture.debugElement.query(By.css('.close')).nativeElement.click();
176176
fixture.detectChanges();
177-
tick();
177+
flush();
178178

179179
expect(testComponent.backdropClickedCount).toBe(1);
180180
}));
@@ -198,7 +198,7 @@ describe('MatDrawer', () => {
198198

199199
dispatchKeyboardEvent(drawer.nativeElement, 'keydown', ESCAPE);
200200
fixture.detectChanges();
201-
tick();
201+
flush();
202202

203203
expect(testComponent.closeCount).toBe(1, 'Expected one close event.');
204204
expect(testComponent.closeStartCount).toBe(1, 'Expected one close start event.');
@@ -261,12 +261,12 @@ describe('MatDrawer', () => {
261261
openButton.focus();
262262
drawer.open();
263263
fixture.detectChanges();
264-
tick();
264+
flush();
265265
drawerButton.focus();
266266

267267
drawer.close();
268268
fixture.detectChanges();
269-
tick();
269+
flush();
270270

271271
expect(document.activeElement)
272272
.toBe(openButton, 'Expected focus to be restored to the open button on close.');

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
TestBed,
78
tick
@@ -506,10 +507,10 @@ describe('MatTooltip', () => {
506507
fixture.detectChanges();
507508
}).not.toThrow();
508509

509-
tick(0);
510+
flush();
510511
}));
511512

512-
it('should not show the tooltip on progammatic focus', fakeAsync(() => {
513+
it('should not show the tooltip on programmatic focus', fakeAsync(() => {
513514
assertTooltipInstance(tooltipDirective, false);
514515

515516
buttonElement.focus();

0 commit comments

Comments
 (0)