From b123af0f4f5a3c2f28abf89678429820b5b42e35 Mon Sep 17 00:00:00 2001 From: Jesse Date: Wed, 20 Sep 2023 12:19:45 -0400 Subject: [PATCH] Only look for focusable PDOM elements if the target is actually under the PDOM, see https://github.com/phetsims/scenery/issues/1550 --- js/accessibility/FocusManager.ts | 19 ++++++++----------- js/display/Display.ts | 7 +++++++ js/input/Input.ts | 15 ++++----------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/js/accessibility/FocusManager.ts b/js/accessibility/FocusManager.ts index 37415728c..39f1ed2dd 100644 --- a/js/accessibility/FocusManager.ts +++ b/js/accessibility/FocusManager.ts @@ -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 ); diff --git a/js/display/Display.ts b/js/display/Display.ts index 7299f0c92..7c3865364 100644 --- a/js/display/Display.ts +++ b/js/display/Display.ts @@ -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, diff --git a/js/input/Input.ts b/js/input/Input.ts index cbc377a64..5d5d919ea 100644 --- a/js/input/Input.ts +++ b/js/input/Input.ts @@ -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 @@ -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! ); @@ -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; } @@ -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; } @@ -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