Don't generally hide contents in the tree as they could be important [chromium/src : master]
Aaron Leventhal (Gerrit)
+dmazzoni PTAL
To view, visit change 615149. To unsubscribe, or for help writing mail filters, visit settings.
Dominic Mazzoni (Gerrit)
Dominic Mazzoni would like Alice Boxhall to review this change.
Don't generally hide contents in the tree as they could be important
Bug: 689204
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib8a5a23755bf77877ab8e196ae4a7435a1d7297a
---
M chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs
M content/test/data/accessibility/aria/aria-activedescendant-expected-blink.txt
M content/test/data/accessibility/aria/aria-button-expected-android.txt
M content/test/data/accessibility/aria/aria-button-expected-blink.txt
M content/test/data/accessibility/aria/aria-button-expected-mac.txt
M content/test/data/accessibility/aria/aria-button-expected-win.txt
M content/test/data/accessibility/aria/aria-button.html
M content/test/data/accessibility/aria/aria-checkbox-expected-android.txt
M content/test/data/accessibility/aria/aria-checkbox-expected-blink.txt
M content/test/data/accessibility/aria/aria-checkbox-expected-mac.txt
M content/test/data/accessibility/aria/aria-checkbox-expected-win.txt
M content/test/data/accessibility/aria/aria-checkbox.html
M content/test/data/accessibility/aria/aria-level-expected-blink.txt
M content/test/data/accessibility/aria/aria-level-expected-mac.txt
M content/test/data/accessibility/aria/aria-level-expected-win.txt
M content/test/data/accessibility/aria/aria-math-expected-android.txt
M content/test/data/accessibility/aria/aria-math-expected-blink.txt
M content/test/data/accessibility/aria/aria-math-expected-mac.txt
M content/test/data/accessibility/aria/aria-math-expected-win.txt
M content/test/data/accessibility/aria/aria-menuitem-expected-android.txt
M content/test/data/accessibility/aria/aria-menuitem-expected-blink.txt
M content/test/data/accessibility/aria/aria-menuitem-expected-mac.txt
M content/test/data/accessibility/aria/aria-menuitem-expected-win.txt
M content/test/data/accessibility/aria/aria-menuitem.html
M content/test/data/accessibility/aria/aria-menuitemcheckbox-expected-blink.txt
M content/test/data/accessibility/aria/aria-menuitemcheckbox-expected-mac.txt
M content/test/data/accessibility/aria/aria-menuitemcheckbox-expected-win.txt
M content/test/data/accessibility/aria/aria-menuitemradio-expected-blink.txt
M content/test/data/accessibility/aria/aria-menuitemradio-expected-mac.txt
M content/test/data/accessibility/aria/aria-menuitemradio-expected-win.txt
M content/test/data/accessibility/aria/aria-separator-expected-android.txt
M content/test/data/accessibility/aria/aria-separator-expected-blink.txt
M content/test/data/accessibility/aria/aria-separator-expected-mac.txt
M content/test/data/accessibility/aria/aria-tab-expected-android.txt
M content/test/data/accessibility/aria/aria-tab-expected-win.txt
M content/test/data/accessibility/aria/aria-tabpanel-expected-android.txt
M content/test/data/accessibility/aria/aria-tabpanel-expected-blink.txt
M content/test/data/accessibility/aria/aria-tabpanel-expected-mac.txt
M content/test/data/accessibility/aria/aria-tabpanel-expected-win.txt
M content/test/data/accessibility/aria/aria-tree-expected-android.txt
M content/test/data/accessibility/aria/aria-tree-expected-blink.txt
M content/test/data/accessibility/aria/aria-tree-expected-mac.txt
M content/test/data/accessibility/aria/aria-tree-expected-win.txt
M content/test/data/accessibility/aria/aria-tree.html
M content/test/data/accessibility/html/input-button-in-menu-expected-blink.txt
M content/test/data/accessibility/html/input-button-in-menu-expected-win.txt
M content/test/data/accessibility/html/input-datetime-local-expected-win.txt
M content/test/data/accessibility/html/input-month-expected-blink.txt
M content/test/data/accessibility/html/input-month-expected-mac.txt
M content/test/data/accessibility/html/input-time-expected-blink.txt
M content/test/data/accessibility/html/input-time-expected-mac.txt
M content/test/data/accessibility/html/input-time-expected-win.txt
M content/test/data/accessibility/html/input-week-expected-blink.txt
M content/test/data/accessibility/html/input-week-expected-mac.txt
M content/test/data/accessibility/html/input-week-expected-win.txt
M third_party/WebKit/LayoutTests/accessibility/nochildren-elements-expected.txt
M third_party/WebKit/LayoutTests/accessibility/nochildren-elements.html
M third_party/WebKit/LayoutTests/inspector-protocol/accessibility/accessibility-ignoredNodes-expected.txt
M third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp
M third_party/WebKit/Source/modules/accessibility/AXLayoutObject.h
M third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
M third_party/WebKit/Source/modules/accessibility/AXObject.cpp
M third_party/WebKit/Source/modules/accessibility/AXObject.h
M third_party/WebKit/Source/modules/accessibility/InspectorTypeBuilderHelper.cpp
64 files changed, 412 insertions(+), 340 deletions(-)
Dominic Mazzoni (Gerrit)
Sometimes I think there ought to be an ARIA attribute that says
to keep this node, but make its children (and the rest of the
subtree) presentational.
One example where I've used role=img before:
<div role="img" aria-label="Big composite image">
<img src="topleft.jpg">
<img src="toprightjpg">
<img src="botleft.jpg">
<img src="botright.jpg">
</div>
Previously adding role=img had the effect of removing the
rest of the subtree, which was perfect in this case. I
think that's sometimes cleaner than adding role=none or
alt="" to every image inside.
Basically, aside from that I'm okay with this and definitely
seems to fix some real-world examples.
Alice, any thoughts?
5 comments:
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:
Have you manually checked this with VoiceOver? What happens
when you navigate between items that are descendants of a
tree (I think an AXOutline on Mac) but some aren't tree items?Isn't the idea of roles with presentational children
part of the ARIA spec, though?
File third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp:
Patch Set #8, Line 2443:
// These roles have ChildrenPresentational: true in the ARIA spec.This logic makes sense, thanks.
Patch Set #8, Line 2620:
// If this is not part of the accessibility then invalidate this object'sdropped "tree" in "accessibility tree"
File third_party/WebKit/Source/modules/accessibility/AXObject.h:
Can we still expose this for the narrow case of an element
with just one text child?
To view, visit change 615149. To unsubscribe, or for help writing mail filters, visit settings.
Nektarios Paisios
Alice Boxhall (Gerrit)
Patch Set 8:
(5 comments)
Sometimes I think there ought to be an ARIA attribute that says
to keep this node, but make its children (and the rest of the
subtree) presentational.One example where I've used role=img before:
<div role="img" aria-label="Big composite image">
<img src="topleft.jpg">
<img src="toprightjpg">
<img src="botleft.jpg">
<img src="botright.jpg">
</div>Previously adding role=img had the effect of removing the
rest of the subtree, which was perfect in this case. I
think that's sometimes cleaner than adding role=none or
alt="" to every image inside.Basically, aside from that I'm okay with this and definitely
seems to fix some real-world examples.Alice, any thoughts?
I've definitely seen cases where people have had to sprinkle role=presentation throughout tree to stop things being chatty/confusing, so I definitely think we should think this through carefully.
But I agree with Dominic that this fixes real issues, and we've definitely been going more towards treating ARIA authoring errors as authoring errors rather than strictly enforcing things.
2 comments:
Patch Set #8, Line 5:
treeitem "non-hidden treeitem"Can you delete these cases from the test, since they are no longer ignored?
File third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp:
typo
To view, visit change 615149. To unsubscribe, or for help writing mail filters, visit settings.
Aaron Leventhal (Gerrit)
@dmazzoni this patch does not change the role="img" case. I put that in it's own case and advised Mozilla to do the same. I just don't see the point of exposing a rich img. I don't imagine authors wanting to do that. And I really think that would confuse users. If you look at the img case it doesn't check whether there is a single text child. It simply never allows children.
I've updated the AriaImg dump tree test case to show this works.
6 comments:
Patch Set #8, Line 5:
treeitem "non-hidden treeitem"Can you delete these cases from the test, since they are no longer ignored?
Done
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:
Isn't the idea of roles with presentational children […]
Yes, as mentioned elsewhere in the CL, this should be pushed to authoring tools, because:
- devs don't always do things perfectly
- it would preclude some useful cases (complex radio buttons, menu items, even buttons as it turns out)
- this aligns us with Firefox
File third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp:
typo
Done
Patch Set #8, Line 2443:
// These roles have ChildrenPresentational: true in the ARIA spec.This logic makes sense, thanks.
Done
Patch Set #8, Line 2620:
// If this is not part of the accessibility then invalidate this object'sdropped "tree" in "accessibility tree"
Can we still expose this for the narrow case of an element […]
I'm not sure what you're asking. In the case of an element with just one text child, we don't ignore anything.
To view, visit change 615149. To unsubscribe, or for help writing mail filters, visit settings.
Aaron Leventhal (Gerrit)
1 comment:
I'm not sure what you're asking. […]
Nevermind. I was tired. My brain was in opposite mode :/
I see what you're asking.
To view, visit change 615149. To unsubscribe, or for help writing mail filters, visit settings.
Aaron Leventhal (Gerrit)
1 comment:
Nevermind. I was tired. My brain was in opposite mode :/ […]
Would we want to use kAXAncestorIsLeafNode ?
To view, visit change 615149. To unsubscribe, or for help writing mail filters, visit settings.
Aaron Leventhal (Gerrit)
@dmazzoni, PTAL
2 comments:
File third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp:
Have you manually checked this with VoiceOver? What happens […]
It doesn't seem any worse, from what I can tell, although I'm not a great VoiceOver user. Stil, the VoiceOver experience with trees seems pretty bad, even in Safari. In Chrome, VoiceOver doesn't announce "expanded" or "collapsed" events (even without this CL).
I think we're going to need to spend some more time on the ARIA+VoiceOver experience overall.
File third_party/WebKit/Source/modules/accessibility/AXObject.h:
Would we want to use kAXAncestorIsLeafNode ?
Implemented with kAncestorIsLeafNode
To view, visit change 615149. To unsubscribe, or for help writing mail filters, visit settings.
Dominic Mazzoni (Gerrit)
Thanks for the img example. lgtm
Patch set 13:Code-Review +1
Aaron Leventhal (Gerrit)
Patch set 13:Commit-Queue +2
Commit Bot (Gerrit)
Commit Bot merged this change.
Don't generally hide contents in the tree as they could be important
Bug: 689204
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib8a5a23755bf77877ab8e196ae4a7435a1d7297a
Reviewed-on: https://chromium-review.googlesource.com/615149
Reviewed-by: Dominic Mazzoni <dmaz...@chromium.org>
Commit-Queue: Aaron Leventhal <aleve...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496284}
---
M chrome/browser/resources/chromeos/chromevox/cvox2/background/output_test.extjs
M content/test/data/accessibility/aria/aria-activedescendant-expected-blink.txt
M content/test/data/accessibility/aria/aria-button-expected-android.txt
M content/test/data/accessibility/aria/aria-button-expected-blink.txt
M content/test/data/accessibility/aria/aria-button-expected-mac.txt
M content/test/data/accessibility/aria/aria-button-expected-win.txt
M content/test/data/accessibility/aria/aria-button.html
M content/test/data/accessibility/aria/aria-checkbox-expected-android.txt
M content/test/data/accessibility/aria/aria-checkbox-expected-blink.txt
M content/test/data/accessibility/aria/aria-checkbox-expected-mac.txt
M content/test/data/accessibility/aria/aria-checkbox-expected-win.txt
M content/test/data/accessibility/aria/aria-checkbox.html
M content/test/data/accessibility/aria/aria-img.html
M content/test/data/accessibility/aria/aria-level-expected-blink.txt
M content/test/data/accessibility/aria/aria-level-expected-mac.txt
M content/test/data/accessibility/aria/aria-level-expected-win.txt
M content/test/data/accessibility/aria/aria-math-expected-android.txt
M content/test/data/accessibility/aria/aria-math-expected-blink.txt
M content/test/data/accessibility/aria/aria-math-expected-mac.txt
M content/test/data/accessibility/aria/aria-math-expected-win.txt
M content/test/data/accessibility/aria/aria-menuitem-expected-android.txt
M content/test/data/accessibility/aria/aria-menuitem-expected-blink.txt
M content/test/data/accessibility/aria/aria-menuitem-expected-mac.txt
M content/test/data/accessibility/aria/aria-menuitem-expected-win.txt
M content/test/data/accessibility/aria/aria-menuitem.html
M content/test/data/accessibility/aria/aria-menuitemcheckbox-expected-blink.txt
M content/test/data/accessibility/aria/aria-menuitemcheckbox-expected-mac.txt
M content/test/data/accessibility/aria/aria-menuitemcheckbox-expected-win.txt
M content/test/data/accessibility/aria/aria-menuitemradio-expected-blink.txt
M content/test/data/accessibility/aria/aria-menuitemradio-expected-mac.txt
M content/test/data/accessibility/aria/aria-menuitemradio-expected-win.txt
M content/test/data/accessibility/aria/aria-separator-expected-android.txt
M content/test/data/accessibility/aria/aria-separator-expected-blink.txt
M content/test/data/accessibility/aria/aria-separator-expected-mac.txt
M content/test/data/accessibility/aria/aria-tab-expected-android.txt
M content/test/data/accessibility/aria/aria-tabpanel-expected-android.txt
M content/test/data/accessibility/aria/aria-tabpanel-expected-blink.txt
M content/test/data/accessibility/aria/aria-tabpanel-expected-mac.txt
M content/test/data/accessibility/aria/aria-tabpanel-expected-win.txt
M content/test/data/accessibility/aria/aria-tree-expected-android.txt
M content/test/data/accessibility/aria/aria-tree-expected-blink.txt
M content/test/data/accessibility/aria/aria-tree-expected-mac.txt
M content/test/data/accessibility/aria/aria-tree-expected-win.txt
M content/test/data/accessibility/aria/aria-tree.html
M content/test/data/accessibility/html/input-button-in-menu-expected-blink.txt
M content/test/data/accessibility/html/input-button-in-menu-expected-win.txt
M content/test/data/accessibility/html/input-datetime-local-expected-win.txt
M content/test/data/accessibility/html/input-month-expected-blink.txt
M content/test/data/accessibility/html/input-month-expected-mac.txt
M content/test/data/accessibility/html/input-time-expected-blink.txt
M content/test/data/accessibility/html/input-time-expected-mac.txt
M content/test/data/accessibility/html/input-time-expected-win.txt
M content/test/data/accessibility/html/input-week-expected-blink.txt
M content/test/data/accessibility/html/input-week-expected-mac.txt
M content/test/data/accessibility/html/input-week-expected-win.txt
M third_party/WebKit/LayoutTests/accessibility/nochildren-elements-expected.txt
M third_party/WebKit/LayoutTests/accessibility/nochildren-elements.html
M third_party/WebKit/LayoutTests/inspector-protocol/accessibility/accessibility-ignoredNodes-expected.txt
M third_party/WebKit/LayoutTests/inspector-protocol/accessibility/accessibility-ignoredNodes.js
M third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp
M third_party/WebKit/Source/modules/accessibility/AXLayoutObject.h
M third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp
M third_party/WebKit/Source/modules/accessibility/AXObject.cpp
M third_party/WebKit/Source/modules/accessibility/AXObject.h
M third_party/WebKit/Source/modules/accessibility/AXSVGRoot.cpp
M third_party/WebKit/Source/modules/accessibility/InspectorTypeBuilderHelper.cpp
66 files changed, 361 insertions(+), 339 deletions(-)