Skip to content

Dont report description in browse mode with reportObjectDescriptions#12917

Merged
feerrenrut merged 7 commits intomasterfrom
fixExcessiveBraille-2-12750
Oct 13, 2021
Merged

Dont report description in browse mode with reportObjectDescriptions#12917
feerrenrut merged 7 commits intomasterfrom
fixExcessiveBraille-2-12750

Conversation

@feerrenrut
Copy link
Copy Markdown
Contributor

@feerrenrut feerrenrut commented Oct 8, 2021 •
edited
Loading

Link to issue number:

Fixes: #12750

Summary of the issue:

Historically the option "Object presentation: Report Object Descriptions" (default: true) has been limited to focus mode and object navigation.
In Report aria-description always #12500 this was changed to report descriptions always.
Historically, the word "object" in the settings category, and the name of this option was supposed to imply "focus mode / object nav specific behavior".

Description of how this pull request fixes the issue:

  • Introduces a way to test braille (not dots, raw text) output.
  • Reverts changes from Report aria-description always #12500 that aimed to make the reportObjectDescriptions behavior consistent between browse and focus modes.
  • Updates the user docs to specify that options in the Object Presentation category don't apply to browse mode.

Testing strategy:

System tests with examples inspired by #12750.

  • Browse mode (nav by down arrow) with reportObjectDescriptions True and False
  • Focus mode (nav by tab) with reportObjectDescriptions True and False

Known issues with pull request:

None

Change log entries:

For Developers:

- Braille output can now be checked in system tests.

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English

@feerrenrut feerrenrut force-pushed the fixExcessiveBraille-2-12750 branch from 1247d14 to 87d129a Compare October 8, 2021 08:33
@feerrenrut feerrenrut marked this pull request as ready for review October 8, 2021 10:02
@feerrenrut feerrenrut requested review from a team as code owners October 8, 2021 10:02
@AppVeyorBot

This comment has been minimized.

Sign in to view
@michaelDCurran
Copy link
Copy Markdown
Member

michaelDCurran commented Oct 11, 2021

I have not yet looked at the exact changes in this pr. But, I want to clarify that report object descriptions was originally created for when reporting focus or object navigation. As opposed to controlFields in text content. So, I don't see this as a simple disctinction between browse and focus mode. Rather it is text verses objects.
The reason this is very important is that contenteditable (focus mode) and browse mode should (where ever possible) report the same thing. But I'll look at the diff now.

@michaelDCurran
Copy link
Copy Markdown
Member

michaelDCurran commented Oct 11, 2021

It looks like this pr is based on pr #12888
so I think I'll wait until that is merged before I try and review this one.

@feerrenrut
Copy link
Copy Markdown
Contributor Author

feerrenrut commented Oct 11, 2021

So, I don't see this as a simple disctinction between browse and focus mode. Rather it is text verses objects.

What is an object from a user's perspective? It's very developer centric terminology, and even then quite overloaded.

@feerrenrut feerrenrut force-pushed the fixExcessiveBraille-2-12750 branch from 2688a2c to c428a4e Compare October 11, 2021 06:47
michaelDCurran
michaelDCurran requested changes Oct 11, 2021
View reviewed changes
lukaszgo1
lukaszgo1 reviewed Oct 11, 2021
View reviewed changes
@lukaszgo1
Copy link
Copy Markdown
Contributor

lukaszgo1 commented Oct 11, 2021

Have you tested specifically STR from #12750 and can confirm that content of the title attribute is no longer shown in braille? If so this should be mentioned in the PR description.

@feerrenrut
Copy link
Copy Markdown
Contributor Author

feerrenrut commented Oct 12, 2021

Have you tested specifically STR from #12750 and can confirm that content of the title attribute is no longer shown in braille? If so this should be mentioned in the PR description.

I have tried this, however the description of the issue on #12750 isn't clear enough for me to be sure. However, there is a test sample with a system test checking braille and speech. This shows the precise issue is resolved. There may be other issues with the Wikipedia sample, they can be raised and resolved separately.

@feerrenrut
Copy link
Copy Markdown
Contributor Author

feerrenrut commented Oct 12, 2021

The reporter has confirmed that their specific case is resolved with this build: #12750 (comment)

@feerrenrut feerrenrut merged commit c6aa771 into master Oct 13, 2021
@feerrenrut feerrenrut deleted the fixExcessiveBraille-2-12750 branch October 13, 2021 08:33
@nvaccessAuto nvaccessAuto added this to the 2021.3 milestone Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelDCurran michaelDCurran michaelDCurran approved these changes

@Qchristensen Qchristensen Awaiting requested review from Qchristensen Qchristensen was automatically assigned from nvaccess/userdocs

+1 more reviewer

@lukaszgo1 lukaszgo1 lukaszgo1 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2021.3

Development

Successfully merging this pull request may close these issues.

Excessive Braille text in web navigation with latest alpha versions

5 participants

@feerrenrut @AppVeyorBot @michaelDCurran @lukaszgo1 @nvaccessAuto