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

NVDA reads screen summary when exiting a dialog #318

Closed
Nancy-Salpepi opened this issue Jan 5, 2023 · 8 comments
Closed

NVDA reads screen summary when exiting a dialog #318

Nancy-Salpepi opened this issue Jan 5, 2023 · 8 comments

Comments

@Nancy-Salpepi
Copy link

Test device
Dell

Operating System
Win10

Browser
Firefox

Problem description
For phetsims/qa#868, every time I exit a dialog with the escape key or when focus is on the 'x' NVDA starts to read the sim description. It will say "Friction is an interactive sim. It changes as you play with it. There is a play area and a control"
I don't typically test NVDA, so feel free to close if it is working as expected.

Steps to reproduce

  1. Turn on NVDA
  2. Tab to and select either the Preferences or Keyboard dialog
  3. Press 'esc' or space bar if 'x' has focus.

Visuals

NVDAfriction.mov
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Friction‬ URL: https://phet-dev.colorado.edu/html/friction/1.6.0-dev.28/phet/friction_all_phet.html Version: 1.6.0-dev.28 2022-12-20 18:30:08 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/108.0.0.0 Safari/537.36 Language: en-US Window: 1475x780 Pixel Ratio: 1.7999999523162842/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 31 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

That is indeed a bug, it should focus and read the name of the preferences nav bar button. I don't see this happening in Ratio and Proportion most likely because of the multi-screen-ness of it. There is logic that manually focuses the beginning of the screen summary, but I wouldn't expect it to be running like this in a single screen sim. I'll take a look.

@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

So the document.activeElement is the close button until this line, and then afterwards it is the document body.

https://github.com/phetsims/joist/blob/dfdaa4a1b162c08e9f0f1008485a65c4e56b2f03/js/Sim.ts#L852

I also thought that it could have to do with this code accidentally getting triggered before the focusOnHideNode was called but that isn't occurring:
https://github.com/phetsims/joist/blob/681c26a5e7de896071887c0023936a023dc34eb9/js/ScreenView.ts#L132-L133

@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

I'm kinda not sure what to do here, though @jessegreenberg may have better ideas about work around. Here is a patch with 4 console logs, as far as I know this code is run synchronously, so the body has focus for just a few miliseconds, but NVDA picks up the change and starts speaking as soon as it is out of the dialog.

---
Index: joist/js/Sim.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/joist/js/Sim.ts b/joist/js/Sim.ts
--- a/joist/js/Sim.ts	(revision 209e7076ccf0d6a046d3655719e3779856341601)
+++ b/joist/js/Sim.ts	(date 1672953854932)
@@ -849,7 +849,9 @@
         this.setPDOMViewsVisible( true );
       }
     }
+    console.log( document.activeElement );
     this.topLayer.removeChild( popup );
+    console.log( document.activeElement );
   }
 
   private resizeToWindow(): void {
Index: sun/js/Popupable.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/sun/js/Popupable.ts b/sun/js/Popupable.ts
--- a/sun/js/Popupable.ts	(revision 0e4f7479857c7ea4b97ec7ba79b4070fc174f9c5)
+++ b/sun/js/Popupable.ts	(date 1672954685861)
@@ -147,7 +147,10 @@
 
       // return focus to the Node that had focus when the Popupable was opened (or the focusOnHideNode if provided)
       if ( this._nodeToFocusOnHide && this._nodeToFocusOnHide.focusable ) {
+        console.log( document.activeElement );
+
         this._nodeToFocusOnHide.focus();
+        console.log( document.activeElement );
       }
     }
 

I also commended out the focus call to the nodeToFocusOnHide and noticed that the original description is exactly the same, so this isn't an interrupting problem: starting with "out of region" and ending with "Play Area and Control"

@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

If I was being the laziest I could be in regards to this issue, I'd make the argument that we do still EVENTUALLY hear the right thing the first half sentence of the screen summary. But I don't think I'm ready to call it a wont fix until I understand better why this is happening on a single screen sim and not a multi-screen sim.

@zepumph zepumph changed the title NVDA reads sim description when exiting a dialog NVDA reads screen summary when exiting a dialog Jan 5, 2023
@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

With this patch, I add another screen to Friction and it 'fixes' the bug. So there is something about it being a single screen sim:

Subject: [PATCH] use isTRangedProperty to allow UnitConversionProperty to access non-ranged Properties, https://github.com/phetsims/axon/issues/425
---
Index: js/friction-main.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/friction-main.js b/js/friction-main.js
--- a/js/friction-main.js	(revision 8db46b39685efe2e28f39b320d1dad2aaba06323)
+++ b/js/friction-main.js	(date 1672956433436)
@@ -43,13 +43,24 @@
 
   // Create and start the sim
   const screenTandem = Tandem.ROOT.createTandem( 'frictionScreen' );
+  const screenTandem2 = Tandem.ROOT.createTandem( 'friction2Screen' );
   new Sim( FrictionStrings.friction.titleStringProperty, [
     new Screen( () => new FrictionModel( LAYOUT_BOUNDS.width, LAYOUT_BOUNDS.height, screenTandem.createTandem( 'model' ) ),
       model => new FrictionScreenView( model, screenTandem.createTandem( 'view' ) ), {
         backgroundColorProperty: new Property( '#fff' ),
         tandem: screenTandem,
+        name: new Property( 'hi' ),
+        createKeyboardHelpNode: () => new FrictionKeyboardHelpContent()
+      }
+    ),
+    new Screen( () => new FrictionModel( LAYOUT_BOUNDS.width, LAYOUT_BOUNDS.height, screenTandem2.createTandem( 'model' ) ),
+      model => new FrictionScreenView( model, screenTandem2.createTandem( 'view' ) ), {
+        backgroundColorProperty: new Property( '#fff' ),
+        tandem: screenTandem2,
+        name: new Property( 'hi' ),
         createKeyboardHelpNode: () => new FrictionKeyboardHelpContent()
       }
     )
+
   ], simOptions ).start();
 } );
\ No newline at end of file

I checked again and it doesn't have anything to do with focus changing, from the screen view visibleProperty or elsewhere. The console logs from #318 (comment) are the same: Close button, body, body, Nav bar button.

@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

Furthermore, if you take a multiscreen sim and add the ?screens=1 query parameter, it is just a single screen, but this bug doesn't occur. In that mode there is no phet.joist.sim.homeScreen, and also no PDOM markup for the nav bar screen buttons.

@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

@zepumph
Copy link
Member

zepumph commented Jan 5, 2023

Ok. So the base issue brought here was that we wish we didn't hear anything but the newly focused item. That is not possible. NVDA will say "out of region" and then the "top" item in the newly visible PDOM when closing the dialog. Then it will next say the newly focused button.

The weirdness of this bug because that for single screen sims with names that are just a single word, the top level h1 is ignored, and the first screen summary sentance-ish is spoken. For all other cases where the h1 is multiple words, includes a comma or emdash, etc, NVDA picks that to speak when closing the dialog.

@jessegreenberg are ready to close this as a won't fix, as we aren't going to change the title of this sim to be multiple words. I also don't see an easy way to submit a bug report to them. Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants