Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cdk/a11y): Fix the touch/program origin regression introduced in the recent FocusMonitor refactor. #22754

Merged
merged 4 commits into from May 25, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 46 additions & 9 deletions src/cdk/a11y/focus-monitor/focus-monitor.ts
Expand Up @@ -96,6 +96,12 @@ export class FocusMonitor implements OnDestroy {
/** The timeout id of the origin clearing timeout. */
private _originTimeoutId: number;

/**
* Whether the origin was determined via a touch interaction. Necessary as properly attributing
* focus events to touch interactions requires special logic.
*/
private _originFromTouchInteraction = false;

/** Map of elements being monitored to their info. */
private _elementInfo = new Map<HTMLElement, MonitoredElementInfo>();

Expand Down Expand Up @@ -302,9 +308,15 @@ export class FocusMonitor implements OnDestroy {
}
}

private _getFocusOrigin(): FocusOrigin {
private _getFocusOrigin(focusEventTarget: HTMLElement | null): FocusOrigin {
if (this._origin) {
return this._origin;
// If the origin was realized via a touch interaction, we need to perform additional checks
// to determine whether the focus origin should be attributed to touch or program.
if (this._originFromTouchInteraction) {
return this._shouldBeAttributedToTouch(focusEventTarget) ? 'touch' : 'program';
} else {
return this._origin;
}
}

// If the window has just regained focus, we can restore the most recent origin from before the
Expand All @@ -319,6 +331,29 @@ export class FocusMonitor implements OnDestroy {
return (this._windowFocused && this._lastFocusOrigin) ? this._lastFocusOrigin : 'program';
}

/**
* Returns whether the focus event should be attributed to touch. Recall that in IMMEDIATE mode, a
* touch origin isn't immediately reset at the next tick (see _setOrigin). This means that when we
* handle a focus event following a touch interaction, we need to determine whether (1) the focus
* event was directly caused by the touch interaction or (2) the focus event was caused by a
* subsequent programmatic focus call triggered by the touch interaction.
* @param focusEventTarget The target of the focus event under examination.
*/
private _shouldBeAttributedToTouch(focusEventTarget: HTMLElement | null): boolean {
// Please note that this check is not perfect. Consider the following edge case:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially copied from

// Note(mmalerba): This implementation is not quite perfect, there is a small edge case.
.

//
// <div #parent tabindex="0">
// <div #child tabindex="0" (click)="#parent.focus()"></div>
// </div>
//
// Suppose there is a FocusMonitor in IMMEDIATE mode attached to #parent. When the user touches
// #child, #parent is programmatically focused. This code will attribute the focus to touch
// instead of program. This is a relatively minor edge-case that can be worked around by using
// focusVia(parent, 'program') to focus #parent.
return (this._detectionMode === FocusMonitorDetectionMode.EVENTUAL) ||
!!focusEventTarget?.contains(this._inputModalityDetector._mostRecentTarget);
}

/**
* Sets the focus classes on the element based on the given focus origin.
* @param element The element to update the classes on.
Expand All @@ -337,11 +372,12 @@ export class FocusMonitor implements OnDestroy {
* function to clear the origin at the end of a timeout. The duration of the timeout depends on
* the origin being set.
* @param origin The origin to set.
* @param isFromInteractionEvent Whether we are setting the origin from an interaction event.
* @param isFromInteraction Whether we are setting the origin from an interaction event.
*/
private _setOrigin(origin: FocusOrigin, isFromInteractionEvent = false): void {
private _setOrigin(origin: FocusOrigin, isFromInteraction = false): void {
this._ngZone.runOutsideAngular(() => {
this._origin = origin;
this._originFromTouchInteraction = (origin === 'touch') && isFromInteraction;

// If we're in IMMEDIATE mode, reset the origin at the next tick (or in `TOUCH_BUFFER_MS` ms
// for a touch event). We reset the origin at the next tick because Firefox focuses one tick
Expand All @@ -350,7 +386,7 @@ export class FocusMonitor implements OnDestroy {
// the event queue. Before doing so, clear any pending timeouts.
if (this._detectionMode === FocusMonitorDetectionMode.IMMEDIATE) {
clearTimeout(this._originTimeoutId);
const ms = ((origin === 'touch') && isFromInteractionEvent) ? TOUCH_BUFFER_MS : 1;
const ms = this._originFromTouchInteraction ? TOUCH_BUFFER_MS : 1;
this._originTimeoutId = setTimeout(() => this._origin = null, ms);
}
});
Expand All @@ -370,11 +406,12 @@ export class FocusMonitor implements OnDestroy {
// If we are not counting child-element-focus as focused, make sure that the event target is the
// monitored element itself.
const elementInfo = this._elementInfo.get(element);
if (!elementInfo || (!elementInfo.checkChildren && element !== getTarget(event))) {
const focusEventTarget = getTarget(event);
if (!elementInfo || (!elementInfo.checkChildren && element !== focusEventTarget)) {
return;
}

this._originChanged(element, this._getFocusOrigin(), elementInfo);
this._originChanged(element, this._getFocusOrigin(focusEventTarget), elementInfo);
}

/**
Expand Down Expand Up @@ -431,7 +468,7 @@ export class FocusMonitor implements OnDestroy {
// The InputModalityDetector is also just a collection of global listeners.
this._inputModalityDetector.modalityDetected
.pipe(takeUntil(this._stopInputModalityDetector))
.subscribe(modality => { this._setOrigin(modality, true /* isFromInteractionEvent */); });
.subscribe(modality => { this._setOrigin(modality, true /* isFromInteraction */); });
}
}

Expand Down Expand Up @@ -493,7 +530,7 @@ export class FocusMonitor implements OnDestroy {
}

/** Gets the target of an event, accounting for Shadow DOM. */
function getTarget(event: Event): HTMLElement|null {
export function getTarget(event: Event): HTMLElement|null {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, this file and the input-modality-detector.ts file both import from one another. I'd like to avoid this if possible. Should I move this small helper to a separate file? If so, where's a good place? @crisbeto

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up moving getTarget to input-modality-detector.ts to resolve the circular dependencies (apparently there's some circular deps golden check), but it still doesn't feel like the right place for this small helper to live.

// If an event is bound outside the Shadow DOM, the `event.target` will
// point to the shadow root so we have to use `composedPath` instead.
return (event.composedPath ? event.composedPath()[0] : event.target) as HTMLElement | null;
Expand Down
10 changes: 10 additions & 0 deletions src/cdk/a11y/input-modality/input-modality-detector.ts
Expand Up @@ -16,6 +16,7 @@ import {
isFakeMousedownFromScreenReader,
isFakeTouchstartFromScreenReader,
} from '../fake-event-detection';
import {getTarget} from '../focus-monitor/focus-monitor';

/**
* The input modalities detected by this service. Null is used if the input modality is unknown.
Expand Down Expand Up @@ -100,6 +101,12 @@ export class InputModalityDetector implements OnDestroy {
return this._modality.value;
}

/**
* The most recently detected input modality event target. Is null if no input modality has been
* detected or if the associated event target is null for some unknown reason.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure in what case the event target would be null, but apparently it's possible (at least per the types).

*/
_mostRecentTarget: HTMLElement | null = null;

/** The underlying BehaviorSubject that emits whenever an input modality is detected. */
private readonly _modality = new BehaviorSubject<InputModality>(null);

Expand All @@ -122,6 +129,7 @@ export class InputModalityDetector implements OnDestroy {
if (this._options?.ignoreKeys?.some(keyCode => keyCode === event.keyCode)) { return; }

this._modality.next('keyboard');
this._mostRecentTarget = getTarget(event);
}

/**
Expand All @@ -137,6 +145,7 @@ export class InputModalityDetector implements OnDestroy {
// Fake mousedown events are fired by some screen readers when controls are activated by the
// screen reader. Attribute them to keyboard input modality.
this._modality.next(isFakeMousedownFromScreenReader(event) ? 'keyboard' : 'mouse');
this._mostRecentTarget = getTarget(event);
}

/**
Expand All @@ -156,6 +165,7 @@ export class InputModalityDetector implements OnDestroy {
this._lastTouchMs = Date.now();

this._modality.next('touch');
this._mostRecentTarget = getTarget(event);
}

constructor(
Expand Down