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

VO often reads responses in incorrect order #192

Closed
Nancy-Salpepi opened this issue Jul 13, 2022 · 19 comments
Closed

VO often reads responses in incorrect order #192

Nancy-Salpepi opened this issue Jul 13, 2022 · 19 comments
Assignees

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air (m1 chip)

Operating System
12.4

Browser
safari + VoiceOver

Problem description
For phetsims/qa#819, VoiceOver doesn't read statements in the order listed in phetsims/qa#819 (comment). VO often reads the IR emitting statement before the IR redirecting statement.

Steps to reproduce
These steps have consistently worked for me:

  1. With VO on, tab to and then press the Start Sunlight button
  2. Tab to the Concentration slider and wait for IR rays to start emitting from the surface
  3. Press the Up Arrow key once

Visuals

emittingfirst.mov
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Greenhouse Effect‬ URL: https://phet-dev.colorado.edu/html/greenhouse-effect/1.1.0-dev.10/phet/greenhouse-effect_all_phet.html Version: 1.1.0-dev.10 2022-07-08 17:41:59 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.5 Safari/605.1.15 Language: en-US Window: 1440x704 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 256 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
@Nancy-Salpepi
Copy link
Author

I also see the wrong order uttered in 1.1.0-dev.9. Sorry I didn't catch it.

@arouinfar
Copy link
Contributor

Thanks @Nancy-Salpepi

This seems pedagogically problematic to me. There is a cause and effect relationship that we want users to pick up on. More greenhouse gases in atmosphere means that more infrared is redirected to (and thus absorbed by) the surface resulting in the surface warming and emitting more infrared.

@jessegreenberg
Copy link
Contributor

Screen readers will speak alerts in whatever order they wish, but we hoped UtteranceQueue would feed content to them so that alerts are made in the order that we add them to the queue. In the sim we could probably fix this by adding a small delay in between each alert we send to the UtteranceQueue. But it would be better if a fix can be made in UtteranceQueue directly.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Jul 27, 2022

Just noting that performance on my old MacOS is to bad to0 run VoiceOver with this sim. VoiceOver only ever says "Busy" while the sim is running. But I am able to test alerts by disabling drawing the waves.

    // Update the view when changes occur to the modelled waves.
    model.wavesChangedEmitter.addListener( () => {
      // wavesCanvasNode.invalidatePaint();
    } );

Further testing on my Mac...I very rarely hear any of the responses after changing the concentration. And when I do I only hear "Less infrared radation redirecting back to surface."

EDIT: Identified as likely my old Mac performing poorly, see phetsims/utterance-queue#87

@jbphet
Copy link
Contributor

jbphet commented Jul 27, 2022

Discussed in the 7/27/2022 meeting, and @arouinfar said that it may be a couple of weeks before the planned interviews occur, so it would be nice to address this one prior to the interviews.

@jessegreenberg
Copy link
Contributor

I have a proposed fix for this in phetsims/utterance-queue#88 but I am not able to test VoiceOver with my Mac. @Nancy-Salpepi would you mind trying the sim on master and seeing if this is improved?

@Nancy-Salpepi
Copy link
Author

Nancy-Salpepi commented Jul 28, 2022

@jessegreenberg things are much improved! If I move the slider and allow all the alerts to play, they are in the correct order (15/15).

I only hear statements out of order if I move the slider in the middle of the statements being uttered. Here is an example:

order.mov

@jessegreenberg I'm assuming this is because VO doesn't interrupt alerts??

@jessegreenberg
Copy link
Contributor

Yea, thats right @Nancy-Salpepi. We might be able to make the "More infrared radiation redirecting back to surface" alert "assertive" so that it interrupts and cancels all previous alerts Ill try it out.

@jessegreenberg
Copy link
Contributor

Done in the above commit, it sounds pretty good to me with NVDA. If a "Warming" or "Cooling" slips into the queue right after the slider is moved, we might hear that in between "{{VALUE}} levels of greenhouse gases" and "More infrared radiation redirecting back to surface." But I think that is OK and it is rare.

@Nancy-Salpepi is sounding better for you in VoiceOver?

@Nancy-Salpepi
Copy link
Author

@jessegreenberg the "More/Less radiation redirection back to surface" statement now interrupts alerts! There is a noise that occurs each time before that particular statement. Not sure how you felt about that.

noisebeforestatement.mov

@Nancy-Salpepi
Copy link
Author

Nancy-Salpepi commented Aug 3, 2022

Things sound a bit odd when I switch to Date mode. The Redirecting statement comes before the description of the time period. For example:

  • Year 2020, selected, radio button, 1 of 4
  • [Noise] More infrared radiation redirecting back to surface
  • Now there are a large number of homes and factories in the observation window.

Let me know if you want a video 🙂

@jessegreenberg
Copy link
Contributor

OK, thanks @Nancy-Salpepi - regarding the sound in #192 (comment) I checked with @terracoda and apparently that is a sound that VO plays. Most likely they are trying to signify that something important just happened with an aria-live=assertive alert. There is no way to prevent it currently.

Regarding #192 (comment), yes you are right... Similarly bad output is happening if you change the time period in date mode. I think there is a different implementation that could keep the "assertive" alert and still work for date mode. It might take an hour to do and would be a bit disruptive to the current code. @arouinfar can you please review the recording in #192 (comment) and comment on if this is something we should do? If that recording is not acceptable we can change the current alert implementation, otherwise Ill revert my change in 102839a.

@jbphet
Copy link
Contributor

jbphet commented Aug 3, 2022

We discussed this in today's design meeting, and it was suggested that we try removing obsolete messages from the utterance queue when possible. @jessegreenberg did some real-time investigation and said that this is indeed a possibility. He'll try it out and then we will re-review.

@jbphet jbphet assigned jbphet and unassigned arouinfar Aug 3, 2022
@jessegreenberg jessegreenberg self-assigned this Aug 3, 2022
@jessegreenberg
Copy link
Contributor

Clearing the utterance queue will not help with this. I verified that the alerts have been given to the screen reader before they can be cleared in this case.

Order of alerts currently:
image

We want to hear all of these, it is just that they are coming through in an odd order. This will require changes to the alerter code.

@jessegreenberg
Copy link
Contributor

OK, a change was made to the description implementation so that we should always hear the observation window scene description before "infrared radiation redirecting" descriptions. This should resolve the comment in #192 (comment).

@Nancy-Salpepi can you please try this situation again and see if it is improved?

@Nancy-Salpepi
Copy link
Author

@jessegreenberg I still hear IR radiation description before scene description

outoforder.mov

@jessegreenberg
Copy link
Contributor

Thanks @Nancy-Salpepi! I can hear the "beep" in your screen recording from the assertive alert so I think this is happening because VO reads the assertive alert and then reads the polite alerts without cancelling them. I am going to go ahead and revert my change in 102839a.

Alerts should come out in the right order. But it is possible that #192 (comment) could still happen.

@Nancy-Salpepi can you please confirm that your previous comment does not happen?

@Nancy-Salpepi
Copy link
Author

@jessegreenberg the alerts are now coming out in the right order 😁

@jessegreenberg
Copy link
Contributor

Hooray! Thanks @Nancy-Salpepi. 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

4 participants