Skip to content

Commit

Permalink
Only look for focusable PDOM elements if the target is actually under…
Browse files Browse the repository at this point in the history
… the PDOM, see #1550
  • Loading branch information
jessegreenberg committed Sep 20, 2023
1 parent def51bd commit b123af0
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 22 deletions.
19 changes: 8 additions & 11 deletions js/accessibility/FocusManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,22 +183,19 @@ export default class FocusManager {
assert && assert( document.activeElement, 'Must be called from focusin, therefore active elemetn expected' );

if ( focus ) {
const uniqueId = document.activeElement!.getAttribute( PDOMUtils.DATA_PDOM_UNIQUE_ID )!;
assert && assert( uniqueId, 'Event target must have a unique ID on its data' );

// TODO: But maybe actually we have some clearing to do here. https://github.com/phetsims/scenery/issues/1550
// Cases like a DOM() HTML element don't have a uniqueID, should they?
if ( !uniqueId ) {
return;
}

// Look for the scenery target under the PDOM
for ( let i = 0; i < displays.length; i++ ) {
const display = displays[ i ];

// null if target is not under the PDOM
const trail = PDOMInstance.uniqueIdToTrail( display, uniqueId );
if ( trail ) {
const activeElement = document.activeElement as HTMLElement;
if ( display.isElementUnderPDOM( activeElement ) ) {
const uniqueId = activeElement.getAttribute( PDOMUtils.DATA_PDOM_UNIQUE_ID )!;
assert && assert( uniqueId, 'Event target must have a unique ID on its data if it is in the PDOM.' );

const trail = PDOMInstance.uniqueIdToTrail( display, uniqueId )!;
assert && assert( trail, 'We must have a trail since the target was under the PDOM.' );

const visualTrail = PDOMInstance.guessVisualTrail( trail, display.rootNode );
if ( visualTrail.lastNode().focusable ) {
FocusManager.pdomFocus = new Focus( display, visualTrail );
Expand Down
7 changes: 7 additions & 0 deletions js/display/Display.ts
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,13 @@ export default class Display {
return this._accessible;
}

/**
* Returns true if the element is in the PDOM. That is only possible if the display is accessible.
*/
public isElementUnderPDOM( element: HTMLElement ): boolean {
return this._accessible && this.pdomRootElement!.contains( element );
}

/**
* Implements a workaround that prevents DOM focus from leaving the Display in FullScreen mode. There is
* a bug in some browsers where DOM focus can be permanently lost if tabbing out of the FullScreen element,
Expand Down
15 changes: 4 additions & 11 deletions js/input/Input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1152,7 +1152,7 @@ export default class Input extends PhetioObject {
public getRelatedTargetTrail( domEvent: FocusEvent | MouseEvent ): Trail | null {
const relatedTargetElement = domEvent.relatedTarget;

if ( relatedTargetElement && this.isTargetUnderPDOM( relatedTargetElement as HTMLElement ) ) {
if ( relatedTargetElement && this.display.isElementUnderPDOM( relatedTargetElement as HTMLElement ) ) {

const relatedTarget = ( domEvent.relatedTarget as unknown as Element );
assert && assert( relatedTarget instanceof window.Element ); // eslint-disable-line no-simple-type-checking-assertions
Expand Down Expand Up @@ -1183,7 +1183,7 @@ export default class Input extends PhetioObject {
else {
const target = ( domEvent.target as unknown as Element );
assert && assert( target instanceof window.Element ); // eslint-disable-line no-simple-type-checking-assertions
if ( target && this.isTargetUnderPDOM( target as HTMLElement ) ) {
if ( target && this.display.isElementUnderPDOM( target as HTMLElement ) ) {
const trailIndices = target.getAttribute( PDOMUtils.DATA_PDOM_UNIQUE_ID );
assert && assert( trailIndices, 'should not be null' );
return PDOMInstance.uniqueIdToTrail( this.display, trailIndices! );
Expand Down Expand Up @@ -1636,7 +1636,7 @@ export default class Input extends PhetioObject {
// if the event target is within the PDOM the AT is sending a fake pointer event to the document - do not
// dispatch this since the PDOM should only handle Input.PDOM_EVENT_TYPES, and all other pointer input should
// go through the Display div. Otherwise, activation will be duplicated when we handle pointer and PDOM events
if ( this.isTargetUnderPDOM( context.domEvent.target as HTMLElement ) ) {
if ( this.display.isElementUnderPDOM( context.domEvent.target as HTMLElement ) ) {
return;
}

Expand Down Expand Up @@ -1665,7 +1665,7 @@ export default class Input extends PhetioObject {
// if the event target is within the PDOM the AT is sending a fake pointer event to the document - do not
// dispatch this since the PDOM should only handle Input.PDOM_EVENT_TYPES, and all other pointer input should
// go through the Display div. Otherwise, activation will be duplicated when we handle pointer and PDOM events
if ( this.isTargetUnderPDOM( context.domEvent.target as HTMLElement ) ) {
if ( this.display.isElementUnderPDOM( context.domEvent.target as HTMLElement ) ) {
return;
}

Expand Down Expand Up @@ -1943,13 +1943,6 @@ export default class Input extends PhetioObject {
}
}

/**
* Returns true if the Display is accessible and the element is a descendant of the Display PDOM.
*/
private isTargetUnderPDOM( element: HTMLElement ): boolean {
return this.display._accessible && this.display.pdomRootElement!.contains( element );
}

/**
* Saves the main information we care about from a DOM `Event` into a JSON-like structure. To support
* polymorphism, all supported DOM event keys that scenery uses will always be included in this serialization. If
Expand Down

0 comments on commit b123af0

Please sign in to comment.