diff options
author | dmazzoni <dmazzoni@chromium.org> | 2016-03-18 01:28:41 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-03-18 08:30:03 +0000 |
commit | 30fc95f3dd93562d7be852205c49bd49e793d84c (patch) | |
tree | 242517f8732baabbf70496252d52d69dcfff0b61 | |
parent | 135b2ed3347a44635ae459396b4f7ab5c49849d9 (diff) | |
download | chromium_src-30fc95f3dd93562d7be852205c49bd49e793d84c.zip chromium_src-30fc95f3dd93562d7be852205c49bd49e793d84c.tar.gz chromium_src-30fc95f3dd93562d7be852205c49bd49e793d84c.tar.bz2 |
Fix support for accessible action verbs and performing the default action.
Accessible objects support a "default action" if something happens when
you click on them. On some platforms we return a localized string like
"press" or "uncheck" indicating what action will occur. This change fixes
a few cases that weren't correct:
1. A few objects had actions but no verb - fix this by adding a verb for
pop-up buttons ("open") and a default verb when nothing else applies
("click").
2. Ensure we always return an action verb when an object has a default
action, and never otherwise.
3. Fix detection of click event listeners - we were using an API that
only returned some; use the right API to get all click-and-mouse-related
event listeners, but exclude the HTML BODY element because lots of
pages have click listeners there and they're rarely what we want.
4. The Windows API accDoDefaultAction should return an error if called on
an object with no default action.
BUG=595222
Review URL: https://codereview.chromium.org/1809573003
Cr-Commit-Position: refs/heads/master@{#381911}
14 files changed, 96 insertions, 54 deletions
diff --git a/content/app/strings/content_strings.grd b/content/app/strings/content_strings.grd index 452730f5..51f16efe 100644 --- a/content/app/strings/content_strings.grd +++ b/content/app/strings/content_strings.grd @@ -526,6 +526,12 @@ below: <message name="IDS_AX_LINK_ACTION_VERB" desc="Verb stating the action that will occur when a link is clicked, as used by accessibility."> jump </message> + <message name="IDS_AX_POP_UP_BUTTON_ACTION_VERB" desc="Verb stating the action that will occur when a pop-up button is clicked, as used by accessibility."> + open + </message> + <message name="IDS_AX_DEFAULT_ACTION_VERB" desc="Verb stating the action that will occur when clicking on a generic clickable object, when we don't know what that action is, as used by accessibility."> + click + </message> <message name="IDS_AX_AM_PM_FIELD_TEXT" desc="Accessible description of the AM/PM field in a date/time control"> AM/PM @@ -606,11 +612,11 @@ below: <message name="IDS_AX_MEDIA_HIDE_CLOSED_CAPTIONS_BUTTON" desc="accessibility role description for hide closed captions button"> hide closed captions </message> - + <message name="IDS_AX_MEDIA_CAST_OFF_BUTTON" desc="accessibility role description for remote playback button"> play on remote device </message> - + <message name="IDS_AX_MEDIA_CAST_ON_BUTTON" desc="accessibility role description for remote playback control button"> control remote playback </message> @@ -682,7 +688,7 @@ below: <message name="IDS_AX_MEDIA_CAST_OFF_BUTTON_HELP" desc="accessibility help description for remote playback button"> play on remote device </message> - + <message name="IDS_AX_MEDIA_CAST_ON_BUTTON_HELP" desc="accessibility help description for remote playback control button"> control remote playback </message> diff --git a/content/browser/accessibility/browser_accessibility_win.cc b/content/browser/accessibility/browser_accessibility_win.cc index 03c199c..1c9ee7f 100644 --- a/content/browser/accessibility/browser_accessibility_win.cc +++ b/content/browser/accessibility/browser_accessibility_win.cc @@ -229,6 +229,10 @@ HRESULT BrowserAccessibilityWin::accDoDefaultAction(VARIANT var_id) { if (!target) return E_INVALIDARG; + // Return an error if it's not clickable. + if (!target->HasStringAttribute(ui::AX_ATTR_ACTION)) + return DISP_E_MEMBERNOTFOUND; + manager()->DoDefaultAction(*target); return S_OK; } diff --git a/content/browser/accessibility/dump_accessibility_tree_browsertest.cc b/content/browser/accessibility/dump_accessibility_tree_browsertest.cc index a89e48e..e74fbef 100644 --- a/content/browser/accessibility/dump_accessibility_tree_browsertest.cc +++ b/content/browser/accessibility/dump_accessibility_tree_browsertest.cc @@ -138,6 +138,10 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityAbbr) { RunHtmlTest(FILE_PATH_LITERAL("abbr.html")); } +IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityActionVerbs) { + RunHtmlTest(FILE_PATH_LITERAL("action-verbs.html")); +} + IN_PROC_BROWSER_TEST_F(DumpAccessibilityTreeTest, AccessibilityAddress) { RunHtmlTest(FILE_PATH_LITERAL("address.html")); } diff --git a/content/child/blink_platform_impl.cc b/content/child/blink_platform_impl.cc index d401f96..f825419 100644 --- a/content/child/blink_platform_impl.cc +++ b/content/child/blink_platform_impl.cc @@ -106,6 +106,8 @@ static int ToMessageID(WebLocalizedString::Name name) { return IDS_AX_DATE_TIME_FIELD_EMPTY_VALUE_TEXT; case WebLocalizedString::AXDayOfMonthFieldText: return IDS_AX_DAY_OF_MONTH_FIELD_TEXT; + case WebLocalizedString::AXDefaultActionVerb: + return IDS_AX_DEFAULT_ACTION_VERB; case WebLocalizedString::AXHeadingText: return IDS_AX_ROLE_HEADING; case WebLocalizedString::AXHourFieldText: @@ -196,6 +198,8 @@ static int ToMessageID(WebLocalizedString::Name name) { return IDS_AX_MINUTE_FIELD_TEXT; case WebLocalizedString::AXMonthFieldText: return IDS_AX_MONTH_FIELD_TEXT; + case WebLocalizedString::AXPopUpButtonActionVerb: + return IDS_AX_POP_UP_BUTTON_ACTION_VERB; case WebLocalizedString::AXRadioButtonActionVerb: return IDS_AX_RADIO_BUTTON_ACTION_VERB; case WebLocalizedString::AXSecondFieldText: diff --git a/content/test/data/accessibility/html/action-verbs-expected-blink.txt b/content/test/data/accessibility/html/action-verbs-expected-blink.txt new file mode 100644 index 0000000..b04a00d --- /dev/null +++ b/content/test/data/accessibility/html/action-verbs-expected-blink.txt @@ -0,0 +1,24 @@ +rootWebArea name='Action verbs' +++div +++++staticText name='Generic div' +++++++inlineTextBox name='Generic div' +++heading name='Heading' +++++staticText name='Heading' +++++++inlineTextBox name='Heading' +++group +++++button action='press' name='Button' +++++link action='jump' name='Link' +++++++staticText action='click' name='Link' +++++++++inlineTextBox name='Link' +++++textField +++++checkBox action='check' +++++checkBox action='uncheck' +++++radioButton action='select' +++switch action='check' name='ARIA Switch' +++group +++++popUpButton action='open' +++++++menuListPopup +++++++++menuListOption action='click' name='Pop-up button' +++div action='click' +++++staticText action='click' name='Div with click handler' +++++++inlineTextBox name='Div with click handler' diff --git a/content/test/data/accessibility/html/action-verbs.html b/content/test/data/accessibility/html/action-verbs.html new file mode 100644 index 0000000..f84cb09 --- /dev/null +++ b/content/test/data/accessibility/html/action-verbs.html @@ -0,0 +1,22 @@ +<!-- +@BLINK-ALLOW:action* +--> +<!doctype html> +<html> +<head> + <title>Action verbs</title> +</head> +<body> + <div>Generic div</div> + <h1>Heading</h1> + <button>Button</button> + <a href="#">Link</a> + <input> + <input type=checkbox> + <input type=checkbox checked> + <input type=radio> + <div tabIndex=0 role="switch">ARIA Switch</div> + <select><option>Pop-up button</select> + <div onclick="alert('success');">Div with click handler</div> +</body> +</html> diff --git a/third_party/WebKit/LayoutTests/accessibility/press-works-on-control-types-expected.txt b/third_party/WebKit/LayoutTests/accessibility/press-works-on-control-types-expected.txt index 81de331..c6333b1 100644 --- a/third_party/WebKit/LayoutTests/accessibility/press-works-on-control-types-expected.txt +++ b/third_party/WebKit/LayoutTests/accessibility/press-works-on-control-types-expected.txt @@ -12,7 +12,7 @@ This tests that when certain control type elements are pressed, a valid event is On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS eventSrcElement == document.getElementById('body') is true +PASS eventSrcElement == document.getElementById('container') is true PASS eventSrcElement == document.getElementById('button') is true PASS eventSrcElement == document.getElementById('tab') is true PASS eventSrcElement == document.getElementById('radio') is true diff --git a/third_party/WebKit/LayoutTests/accessibility/press-works-on-control-types.html b/third_party/WebKit/LayoutTests/accessibility/press-works-on-control-types.html index fa8e8ec..0db05b7 100644 --- a/third_party/WebKit/LayoutTests/accessibility/press-works-on-control-types.html +++ b/third_party/WebKit/LayoutTests/accessibility/press-works-on-control-types.html @@ -3,24 +3,26 @@ <head> <script src="../resources/js-test.js"></script> </head> -<body id="body"> +<body> -<div role="group" id="group" tabindex="0">group</div> -<div role="button" id="button" tabindex="0">button</div> -<div role="tab" id="tab" tabindex="0">tab button</div> -<div role="radio" id="radio" tabindex="0">radio</div> -<div role="checkbox" id="checkbox" tabindex="0">checkbox</div> -<div role="menuitem" id="menuitem" tabindex="0">menu item</div> -<div role="menuitemcheckbox" id="menuitemcheckbox" tabindex="0">menu item checkbox</div> -<div role="menuitemradio" id="menuitemradio" tabindex="0">menu item radio</div> -<div role="listitem" id="listitem" tabindex="0">list item</div> +<div id="container"> + <div role="group" id="group" tabindex="0">group</div> + <div role="button" id="button" tabindex="0">button</div> + <div role="tab" id="tab" tabindex="0">tab button</div> + <div role="radio" id="radio" tabindex="0">radio</div> + <div role="checkbox" id="checkbox" tabindex="0">checkbox</div> + <div role="menuitem" id="menuitem" tabindex="0">menu item</div> + <div role="menuitemcheckbox" id="menuitemcheckbox" tabindex="0">menu item checkbox</div> + <div role="menuitemradio" id="menuitemradio" tabindex="0">menu item radio</div> + <div role="listitem" id="listitem" tabindex="0">list item</div> +</div> <p id="description"></p> <div id="console"></div> <script> - document.getElementById("body").onmousedown = handlePress; + document.getElementById("container").onmousedown = handlePress; var pressCount = 0; var eventSrcElement; @@ -28,9 +30,9 @@ eventSrcElement = e.srcElement; // First press was on the group element. That is not a control type and it should - // have caused the body to be the target element instead of the group element. + // have caused the container to be the target element instead of the group element. if (pressCount == 0) - shouldBeTrue("eventSrcElement == document.getElementById('body')"); + shouldBeTrue("eventSrcElement == document.getElementById('container')"); else if (pressCount == 1) shouldBeTrue("eventSrcElement == document.getElementById('button')"); else if (pressCount == 2) diff --git a/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp b/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp index bf68f5e..3ba0e4e 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp +++ b/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.cpp @@ -1069,31 +1069,6 @@ AXObject* AXLayoutObject::previousOnLine() const // Properties of interactive elements. // -static String queryString(WebLocalizedString::Name name) -{ - return Locale::defaultLocale().queryString(name); -} - -String AXLayoutObject::actionVerb() const -{ - switch (roleValue()) { - case ButtonRole: - case ToggleButtonRole: - return queryString(WebLocalizedString::AXButtonActionVerb); - case TextFieldRole: - return queryString(WebLocalizedString::AXTextFieldActionVerb); - case RadioButtonRole: - return queryString(WebLocalizedString::AXRadioButtonActionVerb); - case CheckBoxRole: - case SwitchRole: - return queryString(isChecked() ? WebLocalizedString::AXCheckedCheckBoxActionVerb : WebLocalizedString::AXUncheckedCheckBoxActionVerb); - case LinkRole: - return queryString(WebLocalizedString::AXLinkActionVerb); - default: - return emptyString(); - } -} - String AXLayoutObject::stringValue() const { if (!m_layoutObject) diff --git a/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.h b/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.h index 4eab54b..8703649 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.h +++ b/third_party/WebKit/Source/modules/accessibility/AXLayoutObject.h @@ -116,7 +116,6 @@ protected: AXObject* previousOnLine() const override; // Properties of interactive elements. - String actionVerb() const override; String stringValue() const override; // ARIA attributes. diff --git a/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp b/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp index 1716a57..1b59c22 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp +++ b/third_party/WebKit/Source/modules/accessibility/AXNodeObject.cpp @@ -685,16 +685,19 @@ Element* AXNodeObject::mouseButtonListener() const if (!node) return 0; - // check if our parent is a mouse button listener if (!node->isElementNode()) node = node->parentElement(); if (!node) return 0; - // FIXME: Do the continuation search like anchorElement does for (Element* element = toElement(node); element; element = element->parentElement()) { - if (element->getAttributeEventListener(EventTypeNames::click) || element->getAttributeEventListener(EventTypeNames::mousedown) || element->getAttributeEventListener(EventTypeNames::mouseup)) + // It's a pretty common practice to put click listeners on the body or document, but that's + // almost never what the user wants when clicking on an accessible element. + if (isHTMLBodyElement(element)) + break; + + if (element->hasEventListeners(EventTypeNames::click) || element->hasEventListeners(EventTypeNames::mousedown) || element->hasEventListeners(EventTypeNames::mouseup) || element->hasEventListeners(EventTypeNames::DOMActivate)) return element; } diff --git a/third_party/WebKit/Source/modules/accessibility/AXObject.cpp b/third_party/WebKit/Source/modules/accessibility/AXObject.cpp index 3800577..4592961 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXObject.cpp +++ b/third_party/WebKit/Source/modules/accessibility/AXObject.cpp @@ -917,7 +917,8 @@ static String queryString(WebLocalizedString::Name name) String AXObject::actionVerb() const { - // FIXME: Need to add verbs for select elements. + if (!actionElement()) + return emptyString(); switch (roleValue()) { case ButtonRole: @@ -933,13 +934,9 @@ String AXObject::actionVerb() const case LinkRole: return queryString(WebLocalizedString::AXLinkActionVerb); case PopUpButtonRole: - // FIXME: Implement. - return String(); - case MenuListPopupRole: - // FIXME: Implement. - return String(); + return queryString(WebLocalizedString::AXPopUpButtonActionVerb); default: - return emptyString(); + return queryString(WebLocalizedString::AXDefaultActionVerb); } } diff --git a/third_party/WebKit/Source/modules/accessibility/AXObject.h b/third_party/WebKit/Source/modules/accessibility/AXObject.h index bec65aa..e45fb1af 100644 --- a/third_party/WebKit/Source/modules/accessibility/AXObject.h +++ b/third_party/WebKit/Source/modules/accessibility/AXObject.h @@ -728,7 +728,7 @@ public: virtual void wordBoundaries(Vector<AXRange>& words) const { } // Properties of interactive elements. - virtual String actionVerb() const; + String actionVerb() const; virtual AccessibilityButtonState checkboxOrRadioValue() const; virtual InvalidState getInvalidState() const { return InvalidStateUndefined; } // Only used when invalidState() returns InvalidStateOther. diff --git a/third_party/WebKit/public/platform/WebLocalizedString.h b/third_party/WebKit/public/platform/WebLocalizedString.h index 38f39a2..30f939d 100644 --- a/third_party/WebKit/public/platform/WebLocalizedString.h +++ b/third_party/WebKit/public/platform/WebLocalizedString.h @@ -44,6 +44,7 @@ struct WebLocalizedString { AXCheckedCheckBoxActionVerb, AXDateTimeFieldEmptyValueText, AXDayOfMonthFieldText, + AXDefaultActionVerb, AXHeadingText, // Deprecated. AXHourFieldText, AXImageMapText, // Deprecated. @@ -89,6 +90,7 @@ struct WebLocalizedString { AXMillisecondFieldText, AXMinuteFieldText, AXMonthFieldText, + AXPopUpButtonActionVerb, AXRadioButtonActionVerb, AXSecondFieldText, AXTextFieldActionVerb, |