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

Add unmapped command to read a summary of aria-details #12364

Merged
merged 13 commits into from May 19, 2021

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented May 5, 2021

Link to issue number:

None

Summary of the issue:

aria-details is a HTML attribute that contains a reference to a HTML element that contains structured details.

aria-details is appropriate when linking to descriptions or annotations that are a bit more complex — rather than a simple text string, it might contain multiple bits of semantic information:

https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Annotations#aria-details_versus_aria-describedby

It would be helpful for NVDA users to be able to read a summary of aria-details before having to navigate through the structured annotation.

Description of how this pull request fixes the issue:

Adds an unmapped command to read a summary of aria-details.

Testing strategy:

System tests that read an example from chrome are implemented.

Known issues with pull request:

None

Change log entries:

New Feature

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@seanbudd seanbudd requested a review from a team as a code owner May 5, 2021 03:32
@seanbudd seanbudd marked this pull request as draft May 5, 2021 03:32
@AppVeyorBot
Copy link

See test results for failed build of commit b874b786bb

@AppVeyorBot
Copy link

See test results for failed build of commit 00c9d557b7

@XLTechie
Copy link
Collaborator

XLTechie commented May 5, 2021 via email

@feerrenrut
Copy link
Contributor

@XLTechie This is part of a much bigger block of work, which we intend to incrementally add support for.

source/browseMode.py Outdated Show resolved Hide resolved
source/controlTypes.py Outdated Show resolved Hide resolved
@XLTechie
Copy link
Collaborator

XLTechie commented May 5, 2021 via email

@AppVeyorBot
Copy link

See test results for failed build of commit 4b3d75fdb5

source/browseMode.py Outdated Show resolved Hide resolved
@seanbudd seanbudd marked this pull request as ready for review May 6, 2021 23:22
@seanbudd seanbudd requested a review from feerrenrut May 10, 2021 04:30
@AppVeyorBot
Copy link

See test results for failed build of commit 0c8809c773

Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Generally looks good. Take care with the C++ suggestions, please verify before relying on them, it seems easy to make mistakes with BSTR: https://docs.microsoft.com/en-us/cpp/atl/programming-with-ccombstr-atl?view=msvc-160

nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp Outdated Show resolved Hide resolved
nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp Outdated Show resolved Hide resolved
tests/system/robot/chromeTests.py Outdated Show resolved Hide resolved
tests/system/robot/chromeTests.py Outdated Show resolved Hide resolved
@seanbudd seanbudd requested a review from feerrenrut May 12, 2021 03:41
@AppVeyorBot
Copy link

@seanbudd seanbudd requested a review from feerrenrut May 14, 2021 01:35
Copy link
Contributor

@feerrenrut feerrenrut left a comment

Choose a reason for hiding this comment

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

Code looks good. Assuming you have tested this in firefox and chrome again since your changes?

Lets talk about a merging strategy for this on Monday.

@seanbudd seanbudd merged commit d6787b8 into master May 19, 2021
@seanbudd seanbudd deleted the structure-annotations branch May 19, 2021 00:41
@nvaccessAuto nvaccessAuto added this to the 2021.2 milestone May 19, 2021
seanbudd added a commit that referenced this pull request May 26, 2021
#12364 added a command to read the summary of the aria-details. To know whether or not an object has details, this should be reported. However, new users might not want to use this feature so #12409 added a config setting to opt-in.

Description of how this pull request fixes the issue:
Discards the "has details" state change message unless the annotations config option is turned on.
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.

None yet

6 participants