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.

Sign up for GitHub

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

Jump to bottom

[bug] screen reader does not read all newly rendered text #4211

Closed
kennyzhen13 opened this issue May 10, 2021 · 7 comments
Closed

[bug] screen reader does not read all newly rendered text #4211

kennyzhen13 opened this issue May 10, 2021 · 7 comments
Assignees
@kennyzhen13
Labels
bug Something isn't working status: needs author feedback This issue requires additional information from the issue author. status: no recent activity This issue requires additional information from the author, but hasn't been updated recently. status: resolved This issue has been merged into main and deployed to canary.

Comments

@kennyzhen13
Copy link
Contributor

Describe the bug

This bug report describes the issue described under Context in the PR description of #4170.

While using MacOSX VoiceOver, if new text is rendered in more than one area, only one of those texts is read aloud by the screen reader. In this specific instance, a button click renders two new texts: (1) a block of text under the button and (2) a toast message pop-up.

To Reproduce
Steps to reproduce the behavior:

  1. Fetch from the branch in fix: copy issue details button on insecure origin target page #4170.
  2. Build and run AI-Web locally.
  3. Turn on screen reader software. I used MacOSX VoiceOver.
  4. Open this webpage.
  5. Click on the Accessibility Insights for Web Dev plug-in.
  6. Turn on the Automated Checks setting.
  7. Click on any error icon.
  8. In the dialog window, click on Copy failure details and take note of the screen reader logs.

CodePen repro example

Expected behavior

Currently unsure about the expected behavior.

Screenshots

Context (please complete the following information)

Are you willing to submit a PR?

Yes

Did you search for similar existing issues?

No

Additional context

The target page must be from an insecure origin (http:// instead of https://)
@kennyzhen13 kennyzhen13 added the bug Something isn't working label May 10, 2021
@ghost ghost added the status: new This issue is new and requires triage by DRI. label May 10, 2021
@ghost ghost assigned lisli1 May 10, 2021
@kennyzhen13 kennyzhen13 mentioned this issue May 10, 2021
Merged
7 tasks
@lisli1 lisli1 added the status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. label May 10, 2021
@ghost ghost assigned ferBonnin May 10, 2021
@ghost
Copy link

ghost commented May 10, 2021

This issue has been marked as ready for team triage; we will triage it in our weekly review and update the issue. Thank you for contributing to Accessibility Insights!

@ghost ghost removed the status: new This issue is new and requires triage by DRI. label May 10, 2021
@ferBonnin
Copy link
Contributor

Reviewing with Peter, let's try not show the Toast at all in this scenario (error due to not secure target)

@ferBonnin ferBonnin added the status: ready for work This issue is ready to be worked on. label May 11, 2021
@ghost ghost removed the status: ready for triage This issue is ready to be triaged by the Accessibility Insights team. label May 11, 2021
@kennyzhen13
Copy link
Contributor Author

@ferBonnin OK I can fix this since it was a part of my PR #4170

@lisli1 lisli1 added the status: active This issue is currently being worked on by someone. label May 12, 2021
@ghost ghost removed the status: ready for work This issue is ready to be worked on. label May 12, 2021
@ferBonnin
Copy link
Contributor

awesome! thanks @kennyzhen13

@kennyzhen13 kennyzhen13 mentioned this issue May 14, 2021
Merged
7 tasks
jalkire pushed a commit that referenced this issue May 19, 2021
@kennyzhen13
fix: prevent toast message visibility when pressing copy failure deta…
0cf0164 
…ils button from an insecure target page (#4234)

#### Details
This PR is a continuation of PR #4170 and addresses the bug described in #4211. 

Previously, from an insecure target page, if a user clicks the "Copy failure details" button then two messages will render: 
1. help message under the button that reads "To copy failure details, first open the Accessibility Insights for Web page." 
2. toast pop-up reading "Failed to copy failure details." 

Now, this PR removes the aforementioned toast pop-up. Example screenshot is shown below.
![helpMessage](https://user-images.githubusercontent.com/2243970/118338631-7937e880-b4cb-11eb-87ea-f368bca41713.png "Help message visible from clicking 'Copy failure details.' Toast message does not appear anymore.")

<!-- Usually a sentence or two describing what the PR changes -->

##### Motivation
Addresses bug #4211 
<!-- This can be as simple as "addresses issue #123" -->

##### Context

<!-- Are there any parts that you've intentionally left out-of-scope for a later PR to handle? -->

<!-- Were there any alternative approaches you considered? What tradeoffs did you consider? -->

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: #4211 
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [x] (UI changes only) Added screenshots/GIFs to description above
- [x] (UI changes only) Verified usability with NVDA/JAWS
@lisli1 lisli1 added the status: resolved This issue has been merged into main and deployed to canary. label Jul 19, 2021
@ghost ghost removed the status: active This issue is currently being worked on by someone. label Jul 19, 2021
@jalkire
Copy link
Contributor

jalkire commented Jul 28, 2021

Hey @kennyzhen13, thanks for fixing this! Feel free to close out the issue if you feel that it is resolved 🙂

@jalkire jalkire added the status: needs author feedback This issue requires additional information from the issue author. label Jul 28, 2021
@ghost
Copy link

ghost commented Jul 28, 2021

The team requires additional author feedback; please review their replies and update this issue accordingly. Thank you for contributing to Accessibility Insights!

@ghost ghost added the status: no recent activity This issue requires additional information from the author, but hasn't been updated recently. label Aug 2, 2021
@ghost
Copy link

ghost commented Aug 2, 2021

This issue has been automatically marked as stale because it is marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. Thank you for contributing to Accessibility Insights!

@ghost ghost closed this as completed Aug 5, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kennyzhen13 kennyzhen13

Labels
bug Something isn't working status: needs author feedback This issue requires additional information from the issue author. status: no recent activity This issue requires additional information from the author, but hasn't been updated recently. status: resolved This issue has been merged into main and deployed to canary.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kennyzhen13 @jalkire @ferBonnin @lisli1