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

Dont report description in browse mode with reportObjectDescriptions #12917

Merged
merged 7 commits into from
Oct 13, 2021

Conversation

feerrenrut
Copy link
Contributor

@feerrenrut feerrenrut commented Oct 8, 2021

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 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.

@michaelDCurran
Copy link
Member

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
Member

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
Contributor Author

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.

source/gui/settingsDialogs.py Outdated Show resolved Hide resolved
user_docs/en/userGuide.t2t Show resolved Hide resolved
@lukaszgo1
Copy link
Contributor

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
Contributor Author

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
Contributor Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Excessive Braille text in web navigation with latest alpha versions
5 participants