Skip to content

Report aria-label and aria-description even when parent has no visible children#16471

Merged
seanbudd merged 8 commits into
nvaccess:masterfrom
SaschaCowley:figure_aria_hidden_children
May 8, 2024
Merged

Report aria-label and aria-description even when parent has no visible children#16471
seanbudd merged 8 commits into
nvaccess:masterfrom
SaschaCowley:figure_aria_hidden_children

Conversation

@SaschaCowley

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #14514

Summary of the issue:

HTML figure elements with no accessible children (EG, all children set to aria-hidden=true) but with an aria-label or aria-description are not reported by NVDA.

Description of user facing changes

Elements with no accessible children but which have a label or description are now reported in browse mode. Note this only applies to elements which are still rendered; if all children are set to display: none, for example, NVDA still does not report the parent element.

Description of development approach

Render a single space in elements with no content but a label or description set.

Testing strategy:

Manually tested in Firefox and Edge, using the example markup provided by the OP.

Known issues with pull request:

N/A

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.

@seanbudd

seanbudd commented May 1, 2024

Copy link
Copy Markdown
Member

is this ready for review?

@SaschaCowley SaschaCowley marked this pull request as ready for review May 1, 2024 07:22
@SaschaCowley SaschaCowley requested a review from a team as a code owner May 1, 2024 07:22
@SaschaCowley SaschaCowley requested a review from seanbudd May 1, 2024 07:22
Comment thread nvdaHelper/vbufBackends/gecko_ia2/gecko_ia2.cpp Outdated
@seanbudd seanbudd added this to the 2024.3 milestone May 2, 2024
@seanbudd seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label May 7, 2024
@seanbudd

seanbudd commented May 7, 2024

Copy link
Copy Markdown
Member

Could you move the changes to the new changes.md for this PR please?

@seanbudd seanbudd added the blocked/merge-conflicts Merge conflicts exist on this PR label May 7, 2024
@seanbudd seanbudd merged commit a1848dd into nvaccess:master May 8, 2024
@LeonarddeR

Copy link
Copy Markdown
Collaborator

I'm afraid this pr has unexpected side effects. Se #16554

}

if ((isInteractive || role == ROLE_SYSTEM_SEPARATOR) && parentNode->getLength() == 0) {
if ((isInteractive || role == ROLE_SYSTEM_SEPARATOR || name || description.has_value()) && parentNode->getLength() == 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should name be changed to name.has_value() here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not exactly, as name is a BSTR, not an optional<wstring>. I have checked that it's non-null and non-empty in my updated code though.

seanbudd pushed a commit that referenced this pull request May 23, 2024
…16585)

Fixes #16554

Summary of the issue:
#16471 introduced a regression whereby many blank lines were reported in browse mode.

Description of user facing changes
Extraneous blank lines are no longer reported in browse mode.

Description of development approach
Updated checks in gecko_ia2.cpp:

Check that the length of name is non-zero (SysStringLen returns 0 if the BSTR passed is null)
Added check that description's value is not the empty string, as checking that it has a value is necessary but not sufficient.
@SaschaCowley SaschaCowley deleted the figure_aria_hidden_children branch May 29, 2024 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked/merge-conflicts Merge conflicts exist on this PR conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. queued for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A <figure>'s aria-label and description is not spoken when it has a child element with aria-hidden="true"

3 participants