AX: checkboxes don't respect indeterminate IDL attribute state

https://bugs.webkit.org/show_bug.cgi?id=231760 rdar://problem/84269311 Reviewed by Chris Fleizach. This patch features two improvements: 1. The `indeterminate` IDL attribute is respected when determining the AX value of native checkboxes 2. Indeterminate values for both native inputs and aria-checked="mixed" ARIA inputs are now properly exposed on iOS Prior to this patch, our determination of `isIndeterminate` was whether the AX object had an `indeterminate="true"` HTML attribute. However, this HTML attribute is non-standard and not used anywhere besides our AX code. This patch removes usage of it in favor of the correct indeterminate determination mechanisms. * LayoutTests/accessibility/aria-checked-mixed-value.html: Remove usage of `indeterminate` HTML attribute. * LayoutTests/accessibility/checkbox-mixed-value-expected.txt: * LayoutTests/accessibility/checkbox-mixed-value.html: Remove usage of `indeterminate` HTML attribute in favor of `indeterminate` IDL attribute. * LayoutTests/platform/ios/TestExpectations: Enable aria-checked-mixed-value.html and checkbox-mixed-value.html. * LayoutTests/platform/ios/accessibility/aria-checked-mixed-value-expected.txt: Added. * LayoutTests/platform/mac/accessibility/aria-checked-mixed-value-expected.txt: Deleted. * LayoutTests/accessibility/aria-checked-mixed-value-expected.txt: Updated. * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::isIndeterminate const): (WebCore::AccessibilityNodeObject::checkboxOrRadioValue const): (WebCore::AccessibilityNodeObject::isNativeCheckboxOrRadio const): Deleted. * Source/WebCore/accessibility/AccessibilityNodeObject.h: * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::checkboxOrRadioValue const): * Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp: (WebCore::AXIsolatedTree::updateNodeProperties): Fix AXTrace typo. * Source/WebCore/html/HTMLInputElement.cpp: (WebCore::HTMLInputElement::setIndeterminate): Send a value change notification to update the accessibility isolated tree cache, and to inform AX clients of the new value. * Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm: (WTR::AccessibilityUIElement::isIndeterminate const): Implement using the existing WebAccessibilityObjectWrapperIOS::accessibilityIsIndeterminate method. Canonical link: https://commits.webkit.org/258314@main
WebKit · Dec 24, 2022 · 4160624 · 4160624
1 parent 5aa94a8
commit 4160624
Show file tree

Hide file tree

Showing 13 changed files with 93 additions and 131 deletions.
diff --git a/LayoutTests/accessibility/aria-checked-mixed-value-expected.txt b/LayoutTests/accessibility/aria-checked-mixed-value-expected.txt
@@ -1,30 +1,24 @@
-Tests whether mixed values are reported properly.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
+This test ensures mixed values are reported correctly.
 
+<div id="element1" role="radio" aria-checked="mixed"></div>
 Role: AXRole: AXRadioButton
 Mixed: false
 
-
-Role: AXRole: AXRadioMenuItem
+<div id="element2" role="menuitemradio" aria-checked="mixed"></div>
+Role: AXRole: AXMenuItem
 Mixed: false
 
-
-Role: AXRole: AXCheckBox
-Mixed: true
-
-
-Role: AXRole: AXCheckMenuItem
+<div id="element3" role="menuitemcheckbox" aria-checked="mixed"></div>
+Role: AXRole: AXMenuItem
 Mixed: true
 
-
+<div id="element4" role="checkbox"></div>
 Role: AXRole: AXCheckBox
-Mixed: true
-
+Mixed: false
 
+<div id="element5" role="checkbox" aria-checked="mixed"></div>
 Role: AXRole: AXCheckBox
-Mixed: false
+Mixed: true
 
 
 PASS successfullyParsed is true

diff --git a/LayoutTests/accessibility/aria-checked-mixed-value.html b/LayoutTests/accessibility/aria-checked-mixed-value.html
@@ -1,41 +1,32 @@
 <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
 <html>
 <head>
-<script src="../resources/js-test-pre.js"></script>
+<script src="../resources/accessibility-helper.js"></script>
+<script src="../resources/js-test.js"></script>
 </head>
-<body id="body">
-
-<div id="content">
+<body>
 
 <div id="element1" role="radio" aria-checked="mixed"></div> <!-- treat as false for radio roles -->
 <div id="element2" role="menuitemradio" aria-checked="mixed"></div> <!-- treat as false for menuitemradio roles -->
-<div id="element3" role="checkbox" aria-checked="mixed"></div>
-<div id="element4" role="menuitemcheckbox" aria-checked="mixed"></div>
-<div id="element5" role="checkbox" indeterminate="true"></div>
-<div id="element6" role="checkbox" indeterminate="false"></div>
-
-</div>
-
-<p id="description"></p>
-<div id="console"></div>
+<div id="element3" role="menuitemcheckbox" aria-checked="mixed"></div>
+<div id="element4" role="checkbox"></div>
+<div id="element5" role="checkbox" aria-checked="mixed"></div>
 
 <script>
-
-    description("Tests whether mixed values are reported properly.");
-
-    if (window.accessibilityController) {
-          for (var i = 1; i < 7; i++) {
-              var element = accessibilityController.accessibleElementById("element" + i);
-              debug("Role: " + element.role);
-              debug("Mixed: " + element.isIndeterminate);
-              debug("\n");
-          }
-
-          document.getElementById("content").style.visibility = "hidden";
+var output = "This test ensures mixed values are reported correctly.\n\n";
+
+if (window.accessibilityController) {
+    for (var i = 1; i < 6; i++) {
+        const id = `element${i}`;
+        output += `${escapeHTML(document.getElementById(id).outerHTML)}\n`;
+        const axElement = accessibilityController.accessibleElementById(id);
+        output += `Role: ${axElement.role}\n`;
+        output += `Mixed: ${axElement.isIndeterminate}\n\n`;
     }
 
+    debug(output);
+}
 </script>
-
-<script src="../resources/js-test-post.js"></script>
 </body>
 </html>
+
diff --git a/LayoutTests/accessibility/checkbox-mixed-value-expected.txt b/LayoutTests/accessibility/checkbox-mixed-value-expected.txt
@@ -1,16 +1,10 @@
-Tests whether mixed values are reported properly on native checkboxes.
+This test ensures mixed values are reported properly on native checkboxes.
 
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-1: AXRole: AXCheckBox
-Indeterminate status: true
-
-2: AXRole: AXCheckBox
-Indeterminate status: false
-
-3: AXRole: AXCheckBox
-Indeterminate status: false
+PASS: accessibilityController.accessibleElementById('checkbox').isIndeterminate === false
+document.getElementById('checkbox').indeterminate = true
+PASS: accessibilityController.accessibleElementById('checkbox').isIndeterminate === true
+document.getElementById('checkbox').indeterminate = false
+PASS: accessibilityController.accessibleElementById('checkbox').isIndeterminate === false
 
 PASS successfullyParsed is true
 

diff --git a/LayoutTests/accessibility/checkbox-mixed-value.html b/LayoutTests/accessibility/checkbox-mixed-value.html
@@ -1,37 +1,34 @@
 <!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
 <html>
 <head>
-<script src="../resources/js-test-pre.js"></script>
+<script src="../resources/accessibility-helper.js"></script>
+<script src="../resources/js-test.js"></script>
 </head>
-<body id="body">
+<body>
 
-<div id="content">
-
-<input type="checkbox" id="element1" indeterminate="true">
-<input type="checkbox" id="element2" indeterminate="false">
-<input type="checkbox" id="element3">
-
-</div>
-
-<p id="description"></p>
-<div id="console"></div>
+<input type="checkbox" id="checkbox">
 
 <script>
+var output = "This test ensures mixed values are reported properly on native checkboxes.\n\n";
 
-    description("Tests whether mixed values are reported properly on native checkboxes.");
+if (window.accessibilityController) {
+    window.jsTestIsAsync = true;
+    output += expect("accessibilityController.accessibleElementById('checkbox').isIndeterminate", "false");
 
-    if (window.accessibilityController) {
-          for (var i = 1; i <= 3; i++) {
-              var element = accessibilityController.accessibleElementById("element" + i);
-              debug(i + ": " + element.role);
-              debug("Indeterminate status: " + element.isIndeterminate + "\n");
-          }
+    output += evalAndReturn("document.getElementById('checkbox').indeterminate = true");
+    setTimeout(async function() {
+        await waitFor(() => accessibilityController.accessibleElementById("checkbox").isIndeterminate);
+        output += expect("accessibilityController.accessibleElementById('checkbox').isIndeterminate", "true");
 
-          document.getElementById("content").style.visibility = "hidden";
-    }
+        output += evalAndReturn("document.getElementById('checkbox').indeterminate = false");
+        await waitFor(() => !accessibilityController.accessibleElementById("checkbox").isIndeterminate);
+        output += expect("accessibilityController.accessibleElementById('checkbox').isIndeterminate", "false");
 
+        debug(output);
+        finishJSTest();
+    }, 0);
+}
 </script>
-
-<script src="../resources/js-test-post.js"></script>
 </body>
 </html>
+
diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
@@ -2141,6 +2141,7 @@ fast/dom/linkify-phone-numbers.html [ Pass ]
 
 accessibility/table-exposure-updates-dynamically.html [ Pass ]
 accessibility/accessibility-node-reparent.html [ Pass ]
+accessibility/aria-checked-mixed-value.html [ Pass ]
 accessibility/aria-describedby-on-input.html [ Pass ]
 accessibility/aria-hidden-display-contents-element.html [ Pass ]
 accessibility/aria-multiline.html [ Pass ]
@@ -2150,6 +2151,7 @@ accessibility/aria-roles-unignored.html [ Pass ]
 accessibility/aria-slider-value.html [ Pass ]
 accessibility/ax-object-destroyed-on-reload.html [ Pass ]
 accessibility/basic-focusability.html [ Pass ]
+accessibility/checkbox-mixed-value.html [ Pass ]
 accessibility/display-contents-dynamically-added-children.html [ Pass ]
 accessibility/display-contents-element-roles.html [ Pass ]
 accessibility/display-contents-object-ordering.html [ Pass ]

diff --git a/LayoutTests/platform/ios/accessibility/aria-checked-mixed-value-expected.txt b/LayoutTests/platform/ios/accessibility/aria-checked-mixed-value-expected.txt
@@ -0,0 +1,27 @@
+This test ensures mixed values are reported correctly.
+
+<div id="element1" role="radio" aria-checked="mixed"></div>
+Role: RadioButton
+Mixed: false
+
+<div id="element2" role="menuitemradio" aria-checked="mixed"></div>
+Role: MenuItemRadio
+Mixed: false
+
+<div id="element3" role="menuitemcheckbox" aria-checked="mixed"></div>
+Role: MenuItemCheckbox
+Mixed: true
+
+<div id="element4" role="checkbox"></div>
+Role: CheckBox
+Mixed: false
+
+<div id="element5" role="checkbox" aria-checked="mixed"></div>
+Role: CheckBox
+Mixed: true
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/platform/mac/accessibility/aria-checked-mixed-value-expected.txt b/LayoutTests/platform/mac/accessibility/aria-checked-mixed-value-expected.txt
diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp b/Source/WebCore/accessibility/AccessibilityNodeObject.cpp
@@ -711,16 +711,6 @@ bool AccessibilityNodeObject::isMenuItem() const
     }
 }
 
-bool AccessibilityNodeObject::isNativeCheckboxOrRadio() const
-{
-    Node* node = this->node();
-    if (!is<HTMLInputElement>(node))
-        return false;
-
-    auto& input = downcast<HTMLInputElement>(*node);
-    return input.isCheckbox() || input.isRadioButton();
-}
-
 bool AccessibilityNodeObject::isEnabled() const
 {
     // ARIA says that the disabled status applies to the current element and all descendant elements.
@@ -744,7 +734,7 @@ bool AccessibilityNodeObject::isEnabled() const
 
 bool AccessibilityNodeObject::isIndeterminate() const
 {
-    return equalLettersIgnoringASCIICase(getAttribute(indeterminateAttr), "true"_s);
+    return supportsCheckedState() && checkboxOrRadioValue() == AccessibilityButtonState::Mixed;
 }
 
 bool AccessibilityNodeObject::isPressed() const
@@ -1066,8 +1056,8 @@ AXCoreObject* AccessibilityNodeObject::selectedTabItem()
 
 AccessibilityButtonState AccessibilityNodeObject::checkboxOrRadioValue() const
 {
-    if (isNativeCheckboxOrRadio())
-        return isIndeterminate() ? AccessibilityButtonState::Mixed : isChecked() ? AccessibilityButtonState::On : AccessibilityButtonState::Off;
+    if (auto* input = dynamicDowncast<HTMLInputElement>(node()); input && (input->isCheckbox() || input->isRadioButton()))
+        return input->indeterminate() ? AccessibilityButtonState::Mixed : isChecked() ? AccessibilityButtonState::On : AccessibilityButtonState::Off;
 
     return AccessibilityObject::checkboxOrRadioValue();
 }

diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.h b/Source/WebCore/accessibility/AccessibilityNodeObject.h
@@ -67,7 +67,6 @@ class AccessibilityNodeObject : public AccessibilityObject {
     bool isMenuItem() const override;
     bool isMenuRelated() const override;
     bool isMultiSelectable() const override;
-    virtual bool isNativeCheckboxOrRadio() const;
     bool isNativeImage() const;
     bool isNativeTextControl() const override;
     bool isPasswordField() const override;

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -3134,7 +3134,7 @@ bool AccessibilityObject::supportsRowCountChange() const
 
 AccessibilityButtonState AccessibilityObject::checkboxOrRadioValue() const
 {
-    // If this is a real checkbox or radio button, AccessibilityRenderObject will handle.
+    // If this is a real checkbox or radio button, AccessibilityNodeObject will handle.
     // If it's an ARIA checkbox, radio, or switch the aria-checked attribute should be used.
     // If it's a toggle button, the aria-pressed attribute is consulted.
 
@@ -3158,9 +3158,6 @@ AccessibilityButtonState AccessibilityObject::checkboxOrRadioValue() const
         return AccessibilityButtonState::Mixed;
     }
 
-    if (isIndeterminate())
-        return AccessibilityButtonState::Mixed;
-
     return AccessibilityButtonState::Off;
 }
 

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
@@ -410,7 +410,7 @@ void AXIsolatedTree::updatePropertiesForSelfAndDescendants(AXCoreObject& axObjec
 
 void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const Vector<AXPropertyName>& properties)
 {
-    AXTRACE("AXIsolatedTree::updateNodeProperty"_s);
+    AXTRACE("AXIsolatedTree::updateNodeProperties"_s);
     AXLOG(makeString("Updating properties ", properties, " for objectID ", axObject.objectID().loggingString()));
     ASSERT(isMainThread());
 

diff --git a/Source/WebCore/html/HTMLInputElement.cpp b/Source/WebCore/html/HTMLInputElement.cpp
@@ -984,6 +984,9 @@ void HTMLInputElement::setIndeterminate(bool newValue)
 
     if (auto* renderer = this->renderer(); renderer && renderer->style().hasEffectiveAppearance())
         renderer->theme().stateChanged(*renderer, ControlStates::States::Checked);
+
+    if (auto* cache = document().existingAXObjectCache())
+        cache->valueChanged(this);
 }
 
 bool HTMLInputElement::sizeShouldIncludeDecoration(int& preferredSize) const

diff --git a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm b/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
@@ -98,6 +98,7 @@ - (BOOL)_accessibilityHasTouchEventListener;
 - (NSString *)accessibilityExpandedTextValue;
 - (NSString *)accessibilitySortDirection;
 - (BOOL)accessibilityIsExpanded;
+- (BOOL)accessibilityIsIndeterminate;
 - (NSUInteger)accessibilityBlockquoteLevel;
 - (NSArray *)accessibilityFindMatchingObjects:(NSDictionary *)parameters;
 - (NSArray<NSString *> *)accessibilitySpeechHint;
@@ -697,7 +698,7 @@ - (CGPathRef)_accessibilityPath;
 
 bool AccessibilityUIElement::isIndeterminate() const
 {
-    return false;
+    return [m_element accessibilityIsIndeterminate];
 }
 
 bool AccessibilityUIElement::isExpanded() const