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

Accessibility regression: unspoken first word selection with JAWS screen reader #65267

Closed
2 tasks done
talksina opened this issue Sep 12, 2024 · 23 comments
Closed
2 tasks done
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@talksina
Copy link

Description

WHAT: accessibility bug (regression) in Gutenberg 19.2 - it was not present in 19.1, occurs with JAWS screen reader version 2024.
When writing a new content -post, page, etc.- and having an empty paragraph block, writing some text. Typing is regular, but then, when selecting text, the first word is never spoken and JAWS says "edit box, empty" instead of saying the currently select word; then, selecting further words, they are spoken correctly. Affected also the ctrl+a ("select all") command.
Current result: when editing text, first character/word selected is not spoken by JAWS, while subsequent text is read regularly.
Expected result: every character/word selected, is regularly pronounced by HAWS screen reader.

Step-by-step reproduction instructions

Pre-requirements: Windows 10 or 11, JAWS screen reader 2024 latest build. Browsers, any browser.

  1. activate Gutenberg 19.2 plugin
  2. create a new content - post, page, it's the same.
  3. type a single character, then select it with shift+arrows. JAWS screen reader won't read it. Let's take as a sample, to write the number "1".
  4. after the "1", write a "2". Then, select the "2", it won't read it. But it'll then read the "1" typed before, as it'll be the second character selected sequentially.
  5. Try the same flow with words. And then, with the "select all" (ctrl+a keystroke) in the single block.

Screenshots, screen recording, code snippet

No response

Environment info

WordPress 6.6.2, Gutenberg plugin 19.2, Windows 10 latest builds and Windows 11 latest build, Chrome and Microsoft Edge latest browser builds. JAWS for Windows 2024 last build. With NVDA screen reader, problems seems not to occur.

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes
@talksina talksina added the [Type] Bug An existing feature does not function as intended label Sep 12, 2024
@t-hamano t-hamano added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Sep 12, 2024
@afercia
Copy link
Contributor

afercia commented Sep 17, 2024

@ellatrix any recent change in the contenteditable that may cause this issue?

@jeryj
Copy link
Contributor

jeryj commented Sep 17, 2024

Potentially caused by #63671 or #64928. I'm going to open some revert PRs so we can test if these might have caused the bug.

@jeryj
Copy link
Contributor

jeryj commented Sep 17, 2024

@ellatrix we've identified that #63671 is causing this bug, specifically these lines that set the content editable of the wrapper to true which causes focus to be moved to the body. Removing that contenteditable switch fixes this issue, but breaks the iOS multiselection.

@afercia
Copy link
Contributor

afercia commented Sep 18, 2024

@jeryj thanks for debugging this issue and identifying the root cause.

@youknowriad @ellatrix if it is confirmed that #63671 introduced this regression, I think it would be good to discuss the process and how some changes are introduced in the editor without proper testing. It's not the first time that playing with writingflow, selection or focus introduces important regressions for assistive technologies and I'm not sure playing so lightly with native features is wise, especially without extensive testing. Thanks.

@talksina
Copy link
Author

Gutenberg 19.3 released and the issue is still there. It's honestly impossible to work this way!

@afercia afercia added the [Priority] High Used to indicate top priority items that need quick attention label Sep 25, 2024
@afercia
Copy link
Contributor

afercia commented Sep 25, 2024

@WordPress/gutenberg-core this appears to be a big barrier for screen reader users. I'd kindly ask to consider a fix for the incoming WordPress 6.7 release. Raised the issue priority and added it to the 6.7 editor tasks.

@getdave getdave moved this to 🗣️ In Discussion / Needs Decision in WordPress 6.7 Editor Tasks Sep 25, 2024
@getdave
Copy link
Contributor

getdave commented Sep 25, 2024

I've added the Editor Tech leads @noisysocks and @kevin940726 to the Issue for visibility.

@getdave
Copy link
Contributor

getdave commented Sep 25, 2024

breaks the iOS multiselection.

Is this a "nice to have" feature? If so then we should revert it and restore the usability for all users until we can find an implementation of "iOS multiselection" that doesn't regress the experience.

@t-hamano
Copy link
Contributor

It's also important to note that #63671 has made the Select All shortcut no longer work in text fields inside blocks.

@afercia
Copy link
Contributor

afercia commented Sep 25, 2024

breaks the iOS multiselection.

Is this a "nice to have" feature? If so then we should revert it and restore the usability for all users until we can find an implementation of "iOS multiselection" that doesn't regress the experience.

For what is worth, I'd totally support a revert until a new, non-breaking, solution for multi selection on iOS is identified.

I'd also like to note that such regressions should be ideally prevented by adding specific tests. If a revert is going to happen, I'd recommend to add tests to prevent this happens again in the future.

@talksina
Copy link
Author

talksina commented Sep 25, 2024

I take the chance to mention @joedolson and @annezazu here.
Accessibility team should be made aware of radical modifications in advance, in order to have them tested BEFORE disasters like this happen - that adding a feature breaks accessibility to another.

@annezazu
Copy link
Contributor

Thanks for the ping here. I'm bummed this has happened and that you're dealing with such intense UX problems. I flag things for every single accessibility team meeting and, in this case, the multiselect change wasn't obvious to me to flag as a major feature for accessibility to work on. Most of the time, I'm trying to flag major pieces of work. I'll continue to work on this but know we are and do try to flag things as much as possible with an awareness that the accessibility team too is limited in capacity and cannot review everything. Please keep testing and sharing reports. For testing anything coming up, I highly recommend looking at PRs associated with the next Gutenberg release milestone.

@jeryj
Copy link
Contributor

jeryj commented Sep 25, 2024

I would support a revert #63671 as well, as multiselect on iOS is a new introduction.

@joedolson
Copy link
Contributor

In my opinion, Gutenberg is where we should expect to break things. What matters is that this does not end up in a WordPress release. Practically speaking, it is not possible for the accessibility team to review every single change prior to commit, and I reject the idea that we bear sole responsibility for that. While this is a significant bug, it is also a very subtle bug, and could easily have been missed in testing.

I will say, however, that when the PR includes the line "The one consequence of doing this is that focus will move from the rich text element in the block to the writing flow wrapper.", that should raise a large red flag for all reviewers that this will need an accessibility review. Anything that causes focus changes has high potential for causing accessibility issues, especially if they are a side effect rather than the intended change.

I also support reverting #63671.

@jeryj
Copy link
Contributor

jeryj commented Sep 25, 2024

Here's a revert: #65414

It needs a review 🙏🏻

@jeryj
Copy link
Contributor

jeryj commented Sep 25, 2024

Reverted the PR that caused this bug.

@jeryj jeryj closed this as completed Sep 25, 2024
@github-project-automation github-project-automation bot moved this from 🗣️ In Discussion / Needs Decision to ✅ Done in WordPress 6.7 Editor Tasks Sep 25, 2024
@afercia
Copy link
Contributor

afercia commented Sep 26, 2024

In my opinion, Gutenberg is where we should expect to break things. What matters is that this does not end up in a WordPress release.

I'd totally agree. I'd think it's time to discuss process on the Gutenberg development. While I'm not against fast iterations and experiments in the plugin, I'm not sure everything that gets merged in Gutenberg should be released with WordPress so lightly. After more than seven years of Gutenberg development I think the editor that gets released with WordPress must be way more stable. I do realize this issue is not the best place for such a discussion though as this should be discussed at a higher level.

@kevin940726
Copy link
Member

FWIW, I think investing time in toolings and testing might help in this case. For instance, it's almost always unintended whenever the focus is moved to <body>. We might be able to automatically catch that during either development or e2e testing. This sort of work or exploration doesn't get much recognition, nor can it easily measure the impact, though.

@jeryj
Copy link
Contributor

jeryj commented Sep 26, 2024

@kevin940726 I’d been considering the same. I was worried about the number of false positives it would emit though. I think it’s worth trying to automatically catch focus loss though, as it is a really common issue.

@ciampo
Copy link
Contributor

ciampo commented Sep 26, 2024

focus handler on the bodyelement that fires a warning only when in the testing environment?

@jeryj
Copy link
Contributor

jeryj commented Sep 26, 2024 via email

@afercia
Copy link
Contributor

afercia commented Sep 26, 2024

For example, when would it be OK for focus to be on the body

When users intentionally click an empty part of the page, there's nothing focused I think.

@talksina
Copy link
Author

talksina commented Oct 4, 2024

There's @t-hamano testing Gutenberg in order to fix #61168 issue, and playing around they noticed the "move blocks" accessibility arrows has been broken in 19.2 and 19.3 Gutenberg releases as well. I didn't notice it because I downgraded immediately.
As it affects selection, I can't exclude that the revert that has been made for solving this 65267 one, will solve the other too.
I'll wait to test the reverted one -next Wednesday?- before open another issue with the moving block arrow detail.
Or @t-hamano feel free to describe it if necessary. But before occupying Github with further issues I'd prefer to verify with next new gutenberg release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Priority] High Used to indicate top priority items that need quick attention [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
Status: Done
10 participants