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

Revert "NVDAObject.presentationType: groupings with position information but no name and no description should still be treated as content I.e. announced in focus ancestry. (#14878)" #15638

Merged
merged 4 commits into from Oct 16, 2023

Conversation

michaelDCurran
Copy link
Member

@michaelDCurran michaelDCurran commented Oct 16, 2023

This reverts commit bcc7f25.

Link to issue number:

Reverts pr #14878
Closes #15324

Summary of the issue:

Traditionally, NvDA has only ever announced groupings in the focus ancestry if they have a name or description.
Recently Microsoft had been placing unlabeled groupings within context menus in Office and other places, to denote visual spacing. Microsoft had also recently been exposing position info (x of y) on these groupings.
In order to take advantage of this new info, through suggestion from Microsoft, in pr #14878 NVDA started announcing these unalabeled groupings, if they had position info. the advantage being that the blind user would then have a much better idea of how these context menu items were grouped, and just how many were in each section.
However, it is very clear based on user feedback, that there are way too many small groupings for the extra speech to outway missing info. There are many groupings in these context menus with only 1 or 2 items, greatly slowing down a user's navigation.

Description of user facing changes

NVDA will again no longer report unlabeled gorupings in the focus ancestory.

Description of development approach

Reverts pr #14878.
this pr also tightens up the detection of labels for groupings and similar controls to ignore names and descriptions that only contain whitespace. This is necessary as in some builds of Office, Microsoft also started exposing a space character on the name of unlabeled groupings in order to force screen readers to announce the unlabeled groupings.

Testing strategy:

Moved up and down the context menu in Microsoft Word, ensuring that unlabeled groupings are no longer reported.
Tabbed through the windows settings app to ensure that labeled groupings are still announced.

Known issues with pull request:

Issue #15324 suggested a new setting for Object Presentation settings which would allow configuring if NVDA announced all groupings / labeled groupings / no groupings. But it primary motivation was to remove the announcement of unlabeled groupings. Thus, reverting pr #14878 seems a more straight forward solution for now. And later one we could consider re-introducing something optional - though I personally am not convinced there is much of a need. The community has been quite vocal about this.
remains a problem in the future, NVDA may also need to specifically filter out

Code Review Checklist:

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

…ion but no name and no description should still be treated as content I.e. announced in focus ancestry. (#14878)"

This reverts commit bcc7f25.
@michaelDCurran michaelDCurran requested a review from a team as a code owner October 16, 2023 22:05
@michaelDCurran michaelDCurran marked this pull request as draft October 16, 2023 22:10
@michaelDCurran michaelDCurran marked this pull request as ready for review October 16, 2023 22:28
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Oct 16, 2023
@@ -287,7 +288,6 @@ eSpeak-NG, LibLouis braille translator, and Unicode CLDR have been updated.
- Script for reporting the destination of a link now reports from the caret / focus position rather than the navigator object. (#14659)
- Portable copy creation no longer requires that a drive letter be entered as part of the absolute path. (#14680)
- If Windows is configured to display seconds in the system tray clock, using ``NVDA+f12`` to report the time now honors that setting. (#14742)
- NVDA will now report unlabeled groupings that have useful position information, such as in recent versions of Microsoft Office 365 menus. (#14878)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is removing a line from the 2023.2 change log which should remain unchanged

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@michaelDCurran It seems this PR has been merged without taking this suggestion into account.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, totally missed this. thanks for calling this out.

user_docs/en/changes.t2t Outdated Show resolved Hide resolved
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved pending changelog fixes

Co-authored-by: Sean Budd <sean@nvaccess.org>
@michaelDCurran michaelDCurran merged commit 9878248 into master Oct 16, 2023
1 check was pending
@michaelDCurran michaelDCurran deleted the revert_pr14878 branch October 16, 2023 23:50
@nvaccessAuto nvaccessAuto added this to the 2024.1 milestone Oct 16, 2023
@Brian1Gaff
Copy link

Brian1Gaff commented Oct 17, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a new setting to object presentation category to exclude reporting of groupings when focussing elements
5 participants