Skip to content

Commit 6eedfad

Browse files
twilcotwilco
committed
AX: VoiceOver does not find focusable splitters when searching for next / previous form control
https://bugs.webkit.org/show_bug.cgi?id=307610 rdar://170187464 Reviewed by Joshua Hoffman. When VoiceOver searches for form controls using AXControlSearchKey, it uses isControl() to match elements. Focusable separators (role="separator" with tabindex) and ARIA spinbuttons (role="spinbutton") were not being found because isControl() didn't include these roles. This change: 1. Adds SpinButton to isControl() since ARIA spinbuttons are interactive form controls 2. Adds focusable splitters to isControl() following the existing supportsRangeValue() pattern * LayoutTests/accessibility/search-for-focusable-separator-and-spinbutton-expected.txt: Added. * LayoutTests/accessibility/search-for-focusable-separator-and-spinbutton.html: Added. * LayoutTests/platform/glib/TestExpectations: * Source/WebCore/accessibility/AXCoreObject.cpp: (WebCore::AXCoreObject::isControl const): (WebCore::AXCoreObject::supportsRangeValue const): * Source/WebCore/accessibility/AXCoreObject.h: (WebCore::AXCoreObject::isFocusableSplitter const): * Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm: (-[WebAccessibilityObjectWrapper accessibilityAttributeNames]): Canonical link: https://commits.webkit.org/308089@main
1 parent 1fe26bd commit 6eedfad

File tree

6 files changed

+100
-9
lines changed

6 files changed

+100
-9
lines changed

‎LayoutTests/accessibility/search-for-focusable-separator-and-spinbutton-expected.txt‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
This tests that focusable separators (splitters) and spinbuttons are found by AXControlSearchKey.
2+
3+
PASS: initialControls.includes('focusable-separator') === true
4+
PASS: initialControls.includes('non-focusable-separator') === false
5+
PASS: initialControls.includes('spinbutton') === true
6+
PASS: initialControls.includes('dynamic-separator') === false
7+
8+
Making dynamic-separator focusable by adding tabindex...
9+
10+
Removing tabindex from focusable-separator...
11+
PASS: finalControls.includes('focusable-separator') === false
12+
PASS: finalControls.includes('dynamic-separator') === true
13+
14+
PASS successfullyParsed is true
15+
16+
TEST COMPLETE
17+
Focusable Separator
18+
Non-Focusable Separator
19+
Spinbutton
20+
Initially Non-Focusable

‎LayoutTests/accessibility/search-for-focusable-separator-and-spinbutton.html‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<script src="../resources/accessibility-helper.js"></script>
5+
<script src="../resources/js-test.js"></script>
6+
</head>
7+
<body>
8+
9+
<div id="focusable-separator" role="separator" tabindex="0" aria-valuenow="50" aria-valuemin="0" aria-valuemax="100">Focusable Separator</div>
10+
<div id="non-focusable-separator" role="separator">Non-Focusable Separator</div>
11+
<div id="spinbutton" role="spinbutton" tabindex="0" aria-valuenow="5" aria-valuemin="0" aria-valuemax="10">Spinbutton</div>
12+
<div id="dynamic-separator" role="separator">Initially Non-Focusable</div>
13+
14+
<script>
15+
var output = "This tests that focusable separators (splitters) and spinbuttons are found by AXControlSearchKey.\n\n";
16+
17+
if (window.accessibilityController) {
18+
window.jsTestIsAsync = true;
19+
20+
var webArea = accessibilityController.rootElement.childAtIndex(0);
21+
22+
function collectControlIds() {
23+
var controls = [];
24+
var searchResult = null;
25+
while (true) {
26+
searchResult = webArea.uiElementForSearchPredicate(searchResult, /* isForward */ true, "AXControlSearchKey", "", /* visibleOnly */ false);
27+
if (!searchResult)
28+
break;
29+
var id = searchResult.domIdentifier;
30+
if (id)
31+
controls.push(id);
32+
}
33+
return controls;
34+
}
35+
36+
var initialControls = collectControlIds();
37+
output += expect("initialControls.includes('focusable-separator')", "true");
38+
output += expect("initialControls.includes('non-focusable-separator')", "false");
39+
output += expect("initialControls.includes('spinbutton')", "true");
40+
output += expect("initialControls.includes('dynamic-separator')", "false");
41+
42+
var finalControls;
43+
output += "\nMaking dynamic-separator focusable by adding tabindex...\n";
44+
document.getElementById("dynamic-separator").setAttribute("tabindex", "0");
45+
setTimeout(async function() {
46+
await waitFor(() => collectControlIds().includes("dynamic-separator") );
47+
48+
output += "\nRemoving tabindex from focusable-separator...\n";
49+
document.getElementById("focusable-separator").removeAttribute("tabindex");
50+
51+
await waitFor(() => {
52+
finalControls = collectControlIds();
53+
return !finalControls.includes("focusable-separator");
54+
});
55+
56+
output += expect("finalControls.includes('focusable-separator')", "false");
57+
output += expect("finalControls.includes('dynamic-separator')", "true");
58+
59+
debug(output);
60+
finishJSTest();
61+
}, 0);
62+
}
63+
</script>
64+
</body>
65+
</html>

‎LayoutTests/platform/glib/TestExpectations‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,7 @@ accessibility/display-contents/search-traversal.html [ Skip ]
952952
accessibility/display-none-descendant-of-relation.html [ Skip ]
953953
accessibility/empty-text-under-element-cached.html [ Skip ]
954954
accessibility/menuitem-is-selected.html [ Skip ]
955+
accessibility/search-for-focusable-separator-and-spinbutton.html [ Skip ]
955956
accessibility/search-traversal-after-role-change.html [ Skip ]
956957
accessibility/table-search-traversal.html [ Skip ]
957958
accessibility/tree-update-with-dirty-layout.html [ Skip ]

‎Source/WebCore/accessibility/AXCoreObject.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,13 +108,16 @@ bool AXCoreObject::isControl() const
108108
case AccessibilityRole::SearchField:
109109
case AccessibilityRole::Slider:
110110
case AccessibilityRole::SliderThumb:
111+
case AccessibilityRole::SpinButton:
111112
case AccessibilityRole::Switch:
112113
case AccessibilityRole::TextArea:
113114
case AccessibilityRole::TextField:
114115
case AccessibilityRole::ToggleButton:
115116
return true;
116117
default:
117-
return isFieldset();
118+
// A focusable splitter (separator with tabindex) is considered a control,
119+
// since it can be interacted with to adjust the value.
120+
return isFieldset() || isFocusableSplitter();
118121
}
119122
}
120123

@@ -1015,7 +1018,7 @@ bool AXCoreObject::supportsRangeValue() const
10151018
|| isSlider()
10161019
|| isScrollbar()
10171020
|| isSpinButton()
1018-
|| (isSplitter() && canSetFocusAttribute())
1021+
|| isFocusableSplitter()
10191022
|| hasAttachmentTag();
10201023
}
10211024

‎Source/WebCore/accessibility/AXCoreObject.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,7 @@ class AXCoreObject : public RefCountedAndCanMakeWeakPtr<AXCoreObject> {
647647
bool isPopUpButton() const { return role() == AccessibilityRole::PopUpButton; }
648648
bool isColorWell() const { return role() == AccessibilityRole::ColorWell; }
649649
bool isSplitter() const { return role() == AccessibilityRole::Splitter; }
650+
bool isFocusableSplitter() const { return isSplitter() && canSetFocusAttribute(); }
650651
bool isToolbar() const { return role() == AccessibilityRole::Toolbar; }
651652
bool isSummary() const { return role() == AccessibilityRole::Summary; }
652653
bool isBlockquote() const { return role() == AccessibilityRole::Blockquote; }

‎Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -945,7 +945,13 @@ - (NSArray *)accessibilityAttributeNames
945945
objectAttributes = popupOrToggleButtonAttrs.get().get();
946946
else if (backingObject->isButton())
947947
objectAttributes = buttonAttrs.get().get();
948-
else if (backingObject->isControl())
948+
// Spinbuttons have their own attributes, so check before the generic isControl().
949+
else if (backingObject->isSpinButton()) {
950+
if (backingObject->spinButtonType() == SpinButtonType::Composite)
951+
objectAttributes = compositeSpinButtonAttributes.get().get();
952+
else
953+
objectAttributes = spinButtonCommonAttributes.get().get();
954+
} else if (backingObject->isControl())
949955
objectAttributes = controlAttrs.get().get();
950956

951957
else if (backingObject->isGroup() || backingObject->isListItem() || backingObject->role() == AccessibilityRole::Figure)
@@ -954,12 +960,7 @@ - (NSArray *)accessibilityAttributeNames
954960
objectAttributes = tabListAttrs.get().get();
955961
else if (backingObject->isScrollArea())
956962
objectAttributes = scrollViewAttrs.get().get();
957-
else if (backingObject->isSpinButton()) {
958-
if (backingObject->spinButtonType() == SpinButtonType::Composite)
959-
objectAttributes = compositeSpinButtonAttributes.get().get();
960-
else
961-
objectAttributes = spinButtonCommonAttributes.get().get();
962-
} else if (backingObject->isMenu())
963+
else if (backingObject->isMenu())
963964
objectAttributes = menuAttrs.get().get();
964965
else if (backingObject->isMenuBar())
965966
objectAttributes = menuBarAttrs.get().get();

0 commit comments

Comments
 (0)