Skip to content

Fix double speaking label in angular checkbox#10552

Merged
feerrenrut merged 5 commits into
masterfrom
fixDoubleSpeakingMaterialUICheckbox
Dec 3, 2019
Merged

Fix double speaking label in angular checkbox#10552
feerrenrut merged 5 commits into
masterfrom
fixDoubleSpeakingMaterialUICheckbox

Conversation

@feerrenrut

Copy link
Copy Markdown
Contributor

Link to issue number:

https://bugs.chromium.org/p/chromium/issues/detail?id=995603

Summary of the issue:

The HTML for checkboxes on https://dart-lang.github.io/angular_components/#/material_checkbox have aria-labelledby pointing to a sub-element of the checkbox. Normally, we announce the accessible name as we descend then read the child elements. This results in double speaking the name.

Description of how this pull request fixes the issue:

When rendering the virtual buffer, track the ID's of the nodes. If this node has a label, check if the ID for the label node matches any sub-node then set an attribute labelledByContents. Then in speech.py don't speak the name for a controlField if it has this attribute.

Testing performed:

Simplified test case:

data:text/html,<button>begin</button><div tabindex="0" role="checkbox" aria-labelledby="inner-label"><div style="display:inline" id="inner-label">Simulate evil cat</div></div>

Known issues with pull request:

Change log entry:

Section: New features, Changes, Bug fixes

@feerrenrut feerrenrut self-assigned this Nov 27, 2019
feerrenrut added a commit that referenced this pull request Nov 27, 2019
Example from "Fix double speaking label in angular checkbox" #10552

@michaelDCurran michaelDCurran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't quite get what parentIDs does? I get that outIds stores all descendant IDs, and that if the labelledID appears in the list, then you mark it as the label coming from content.
An out of the box idea though:
Since the buffer already stores nodes by ID, rather than using your own ID lists, why not just make use of what the buffer has already. So call GetControlfieldNodeByIdentifier with the label ID, once before all the possible recursions, and then once after, at the end. If a node did not exist before, but does after, than the node is a descendant. Note that getControlFieldNodeByIdentifier is very cheep as it itself just looks up the ID in a map.

@feerrenrut

Copy link
Copy Markdown
Contributor Author

I don't quite get what parentIDs does?

While developing this I wasn't sure if I would need to check parent ID's on the child side, and left it in for the symmetry.

I see what you are suggesting, with getControlFieldNodeWithIdentifier, it's a clever approach, but I think it makes the code harder to maintain, since it implicitly depends on how that map is built. Though, looking at how that map is built (via the calls to addControlFieldNode), in the same place I'll build an ID tree.

During addControlFieldNode a mapping
of parent->list[immediate children IDs] is built.

This can then be used to get all the children for a node, or all parents
for a node if necessary.
The node tree already has all the information necessary, so removing
the node ID map.
@feerrenrut feerrenrut merged commit 12aef7b into master Dec 3, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Dec 3, 2019
feerrenrut added a commit that referenced this pull request Dec 3, 2019
feerrenrut added a commit that referenced this pull request Dec 4, 2019
- Also fix a typo in #10552
feerrenrut added a commit that referenced this pull request Dec 4, 2019
- Also fix a typo in #10552
feerrenrut added a commit that referenced this pull request Dec 4, 2019
@feerrenrut feerrenrut deleted the fixDoubleSpeakingMaterialUICheckbox branch January 17, 2020 08:55
@maniekins

Copy link
Copy Markdown

I noticed, the aria-labelledby is not working with inner elements after 2019.3 version and it is probably related with this changes. I tested example 2 from https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-labelledby_attribute , added some button next to header, and when we are focusing on that button, main is vocalized but without h1 label.

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.

4 participants