Issue with focus events emitting Display.userGestureEmitter

[jessegreenberg]

userGestureEmitter is used to enable sound and maybe other features in response to legitimate user input. It is emitted on various input events including focus. But We call focus (on the ScreenView for screen reader reasons) on sim startup before any real user input. So it looks to scenery like the userGestureEmitter always emits on sim startup. Since it is always firing on startup I wouldn't be surprised if usages of userGestureEmitter are not working.

[jessegreenberg]

I had hoped that we could determine if the focus event had the trusted flag to determine if it was from script of user input but trusted is true even when calling focus() from javascript.

[jessegreenberg]

I found this https://stackoverflow.com/questions/6727005/determine-what-triggered-focus-event, but the recommended originalEvent is not standard and only works on firefox.

[jessegreenberg]

The documentation for isTrusted

The isTrusted read-only property of the Event interface is a boolean value that is true when the event was generated by a user action, and false when the event was created or modified by a script or dispatched via EventTarget.dispatchEvent().

So I am surprised that it is true in this case.

[jessegreenberg]

I am currently thinking we should only emit the userGestureEmitter on 'input', 'change', 'click', 'keydown', 'keyup' since focusin and focusout could happen from javascript and still be considered "trusted".

[jessegreenberg]

This is the change I want to use:

```patch Index: js/accessibility/pdom/PDOMUtils.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/accessibility/pdom/PDOMUtils.js b/js/accessibility/pdom/PDOMUtils.js --- a/js/accessibility/pdom/PDOMUtils.js (revision 8b47d772adebb09c8505c92a9dbd3ae0612cdcc9) +++ b/js/accessibility/pdom/PDOMUtils.js (date 1634763213543) @@ -65,6 +65,10 @@ // see Input.PDOM_EVENT_TYPES const DOM_EVENTS = [ 'focusin', 'focusout', 'input', 'change', 'click', 'keydown', 'keyup' ]; +// DOM events that must have been triggered from user input of some kind, and will trigger the +// Display.userGestureEmitter. focus and blur events will trigger from scripting so they must be excluded. +const USER_GESTURE_EVENTS = [ 'input', 'change', 'click', 'keydown', 'keyup' ]; + // A collection of DOM events which should be blocked from reaching the scenery Display div // if they are targeted at an ancestor of the PDOM. Some screen readers try to send fake // mouse/touch/pointer events to elements but for the purposes of Accessibility we only @@ -621,6 +625,7 @@ INPUT_TYPES_THAT_SUPPORT_CHECKED: [ 'RADIO', 'CHECKBOX' ], DOM_EVENTS: DOM_EVENTS, + USER_GESTURE_EVENTS: USER_GESTURE_EVENTS, BLOCKED_DOM_EVENTS: BLOCKED_DOM_EVENTS, DATA_TRAIL_ID: DATA_TRAIL_ID, Index: js/input/Input.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/input/Input.js b/js/input/Input.js --- a/js/input/Input.js (revision 8b47d772adebb09c8505c92a9dbd3ae0612cdcc9) +++ b/js/input/Input.js (date 1634763213534) @@ -1051,7 +1051,12 @@ * @param {boolean} bubbles */ dispatchPDOMEvent( trail, eventType, domEvent, bubbles ) { - Display.userGestureEmitter.emit(); + + // exclude focus and blur events because they can happen with scripting without user input, other events + // will have an isTrusted flag to indicate whether they came from scripting or user input + if ( PDOMUtils.USER_GESTURE_EVENTS.includes( eventType ) && domEvent.isTrusted ) { + Display.userGestureEmitter.emit(); + } // This workaround hopefully won't be here forever, see ParallelDOM.setExcludeLabelSiblingFromInput() and https://github.com/phetsims/a11y-research/issues/156 if ( !( domEvent.target && domEvent.target.hasAttribute( PDOMUtils.DATA_EXCLUDE_FROM_INPUT ) ) ) { ```

I want to verify that the isTrusted check works on all platforms and when using screen readers. To test this I will use each screen reader without tabbing into the sim and try moving a slider. userGestureEmitter is currently only used by Sound.js and soundManager.js.

isTrusted may not be true when fuzzing (creating fake input events) so maybe one risk with this is that we will not be able to test sounds with ?fuzzBoard?

[jessegreenberg]

I tested these platforms with the above patch:

• click, keydown, keyup, input, and change are all trusted when NVDA and Firefox and therefore we successfully emit userGestureEmitter from AT input.
• click, keydown, keyup, input, and change are all trusted when using VO and Safari and we succesffully emit the userGestureEmitter from AT input.
• click, keydown, keyup, input and change events are all trusted when using JAWS and Chrome and we correctly emit the userGestureEmitter from AT input
• click, keydown, keyup, input and change events are all trusted when using VO and iOS Safari and we correctly emit the userGestureEmitter from AT input

Its looking good to me!

[jessegreenberg]

Local aqua looks OK so I committed this change. @zepumph do you have other concerns with this or can you think of other things that we may need to test?

[zepumph]

This makes total sense to me. Thanks for doing this. I completely agree. @jessegreenberg and I think that it isn't worth the risk to add the isTrusted check. Let's remove that.

[jessegreenberg]

In the above commit I removed the isTrusted check and did some light spot checking. That was the last item for this issue, closing.