Skip to content

Commit

Permalink
Remove button_drop_down accessibility role
Browse files Browse the repository at this point in the history
It's not necessary, as a role of "button" with the
"has pop up" state is fully supported and sufficient.
The button_drop_down role wasn't even being mapped
correctly on Windows, leading to inconsistent feedback
with some screen readers.

Bug: 766856

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib7f631a759ab44ffc3307204bcc74a9471afc22a
Reviewed-on: https://chromium-review.googlesource.com/842475
Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531363}
  • Loading branch information
minorninth authored and Commit Bot committed Jan 23, 2018
1 parent 44ca004 commit 5a82fc5
Show file tree
Hide file tree
Showing 8 changed files with 2 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ AutomationPredicate.focused = function(node) {
*/
AutomationPredicate.leaf = function(node) {
return !node.firstChild || node.role == Role.BUTTON ||
node.role == Role.BUTTON_DROP_DOWN || node.role == Role.POP_UP_BUTTON ||
node.role == Role.POP_UP_BUTTON ||
node.role == Role.SLIDER || node.role == Role.TEXT_FIELD ||
node.state[State.INVISIBLE] || node.children.every(function(n) {
return n.state[State.INVISIBLE];
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/views/toolbar/toolbar_button.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ void ToolbarButton::OnGestureEvent(ui::GestureEvent* event) {

void ToolbarButton::GetAccessibleNodeData(ui::AXNodeData* node_data) {
Button::GetAccessibleNodeData(node_data);
node_data->role = ui::AX_ROLE_BUTTON_DROP_DOWN;
node_data->role = ui::AX_ROLE_BUTTON;
node_data->AddState(ui::AX_STATE_HASPOPUP);
if (enabled()) {
node_data->AddIntAttribute(ui::AX_ATTR_DEFAULT_ACTION_VERB,
Expand Down
1 change: 0 additions & 1 deletion chrome/common/extensions/api/automation.idl
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
banner,
blockquote,
button,
buttonDropDown,
canvas,
caption,
caret,
Expand Down
3 changes: 0 additions & 3 deletions content/app/strings/content_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,6 @@ below:
<message name="IDS_AX_ROLE_BUTTON" desc="Accessibility role description for a button">
button
</message>
<message name="IDS_AX_ROLE_BUTTON_DROP_DOWN" desc="Accessibility role description for a button that drops down">
drop down button
</message>
<message name="IDS_AX_ROLE_CELL" desc="Accessibility role description for a cell, like in a table or grid">
cell
</message>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -502,9 +502,6 @@ base::string16 BrowserAccessibilityAndroid::GetRoleDescription() const {
case ui::AX_ROLE_BUTTON:
message_id = IDS_AX_ROLE_BUTTON;
break;
case ui::AX_ROLE_BUTTON_DROP_DOWN:
message_id = IDS_AX_ROLE_BUTTON_DROP_DOWN;
break;
case ui::AX_ROLE_CANVAS:
// No role description.
break;
Expand Down
1 change: 0 additions & 1 deletion third_party/closure_compiler/externs/automation.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ chrome.automation.RoleType = {
BANNER: 'banner',
BLOCKQUOTE: 'blockquote',
BUTTON: 'button',
BUTTON_DROP_DOWN: 'buttonDropDown',
CANVAS: 'canvas',
CAPTION: 'caption',
CARET: 'caret',
Expand Down
1 change: 0 additions & 1 deletion ui/accessibility/ax_enums.idl
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
banner,
blockquote,
button,
button_drop_down, // Not used on Web.
canvas,
caption,
caret,
Expand Down
1 change: 0 additions & 1 deletion ui/accessibility/platform/ax_platform_node_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2710,7 +2710,6 @@ int AXPlatformNodeWin::MSAARole() {

// TODO(dmazzoni): figure out the proper MSAA role for roles listed below.
case AX_ROLE_BLOCKQUOTE:
case AX_ROLE_BUTTON_DROP_DOWN:
case AX_ROLE_CARET:
case AX_ROLE_CLIENT:
case AX_ROLE_DEFINITION:
Expand Down

0 comments on commit 5a82fc5

Please sign in to comment.