Skip to content
Permalink
Browse files
AX: indeterminate progress elements are not reported as indeterminate…
… by VoiceOver

https://bugs.webkit.org/show_bug.cgi?id=249875
rdar://problem/103690774

Reviewed by Chris Fleizach.

This patch features two improvements:

  1. On both macOS and iOS, VoiceOver will now properly read
     indeterminate native `progress` elements as "indeterminate"
  2. On iOS, VoiceOver will now properly read indeterminate ARIA
    `role="progressbar"` elements as "indeterminate"

* LayoutTests/accessibility/progress-indeterminate-value-expected.txt: Added.
* LayoutTests/accessibility/progress-indeterminate-value.html: Added.
* LayoutTests/platform/ios/TestExpectations:
Enable new test.
* LayoutTests/platform/win/TestExpectations:
Disable new test.
* Source/WebCore/accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::isIndeterminate const):
Handle indeterminate role="progressbar" elements.
* Source/WebCore/accessibility/AccessibilityObject.h:
(WebCore::AccessibilityObject::hasARIAValueNow const):
Make non-virtual since AXIsolatedObjects have no use for this method.
* Source/WebCore/accessibility/AccessibilityObjectInterface.h:
* Source/WebCore/accessibility/AccessibilityProgressIndicator.cpp:
(WebCore::AccessibilityProgressIndicator::isIndeterminate const):
Added this override for `progress` elements without a `value` attribute.
* Source/WebCore/accessibility/AccessibilityProgressIndicator.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::initializeProperties):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:
Delete `AXPropertyName::HasARIAValueNow` and add
`AXPropertyName::IsIndeterminate` since usage of the former is replaced by
the latter in WebAccessibilityObjectWrapperMac.mm.
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
* Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h:
* Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::isIndeterminate const):
Use `AXCoreObject::isIndeterminate` rather than checking for a integer
value of 2 (which is correct for indeterminate checkboxes but not
progress elements).

Canonical link: https://commits.webkit.org/258332@main
  • Loading branch information
twilco committed Dec 26, 2022
1 parent c8c9622 commit ece8b0ebf62d39388fd82be0ef2a5e66e5c20b34
Show file tree
Hide file tree
Showing 15 changed files with 92 additions and 22 deletions.
@@ -0,0 +1,16 @@
This test ensures that an indeterminate value is properly reported for native and ARIA progress elements.

PASS: accessibilityController.accessibleElementById('native-progress').isIndeterminate === false
PASS: accessibilityController.accessibleElementById('aria-progress').isIndeterminate === false
document.getElementById('native-progress').removeAttribute('value')
document.getElementById('aria-progress').removeAttribute('aria-valuenow')
PASS: accessibilityController.accessibleElementById('native-progress').isIndeterminate === true
PASS: accessibilityController.accessibleElementById('aria-progress').isIndeterminate === true
document.getElementById('aria-progress').setAttribute('role', 'group')
PASS: accessibilityController.accessibleElementById('aria-progress').isIndeterminate === false

PASS successfullyParsed is true

TEST COMPLETE


@@ -0,0 +1,42 @@
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="../resources/accessibility-helper.js"></script>
<script src="../resources/js-test.js"></script>
</head>
<body>

<progress id="native-progress" value="70" max="100"></progress>

<div id="aria-progress" role="progressbar"
aria-valuenow="70" aria-valuemax="100"
style="width: 200px; height: 20px; background-color: blue; margin-top: 30px">
</div>

<script>
var output = "This test ensures that an indeterminate value is properly reported for native and ARIA progress elements.\n\n";

if (window.accessibilityController) {
window.jsTestIsAsync = true;
// A progress element with a value should not be considered indeterminate.
output += expect("accessibilityController.accessibleElementById('native-progress').isIndeterminate", "false");
output += expect("accessibilityController.accessibleElementById('aria-progress').isIndeterminate", "false");

output += evalAndReturn("document.getElementById('native-progress').removeAttribute('value')");
output += evalAndReturn("document.getElementById('aria-progress').removeAttribute('aria-valuenow')");
setTimeout(async function() {
output += await expectAsync("accessibilityController.accessibleElementById('native-progress').isIndeterminate", "true");
output += await expectAsync("accessibilityController.accessibleElementById('aria-progress').isIndeterminate", "true");

// ARIA progress elements should stop being indeterminate if they lose the "progressbar" role.
output += evalAndReturn("document.getElementById('aria-progress').setAttribute('role', 'group')");
output += await expectAsync("accessibilityController.accessibleElementById('aria-progress').isIndeterminate", "false");

debug(output);
finishJSTest();
}, 0);
}
</script>
</body>
</html>

@@ -2168,6 +2168,7 @@ accessibility/multiple-attribute-change-crash.html [ Pass ]
accessibility/node-only-inert-object.html [ Pass ]
accessibility/node-only-object-element-rect.html [ Pass ]
accessibility/placeholder.html [ Pass ]
accessibility/progress-indeterminate-value.html [ Pass ]
accessibility/slider-with-lost-renderer.html [ Pass ]
accessibility/spinbutton-increment-decrement.html [ Pass ]
accessibility/text-under-display-contents-element.html [ Pass ]
@@ -756,6 +756,9 @@ accessibility/custom-elements/ [ Skip ]
# Missing necessary testing logic (AccessibilityController::{get,set}RetainedElement)
accessibility/ax-object-destroyed-on-reload.html [ Skip ]

# Missing AccessibilityUIElement::isIndeterminate() implementation.
accessibility/progress-indeterminate-value.html [ Skip ]

accessibility/display-contents-dynamically-added-children.html [ Skip ]

# DOM paste access requests are not implemented in WebKit1.
@@ -523,6 +523,9 @@ accessibility/focusable-ancestor.html [ Skip ]
# Missing AccessibilityUIElement::{editableAncestor, highestEditableAncestor} implementations.
accessibility/editable-ancestor.html [ Skip ]

# Missing AccessibilityUIElement::isIndeterminate() implementation.
accessibility/progress-indeterminate-value.html [ Skip ]

# Failing since added in https://bugs.webkit.org/show_bug.cgi?id=241428.
accessibility/aria-multiline.html [ Skip ]

@@ -734,7 +734,14 @@ bool AccessibilityNodeObject::isEnabled() const

bool AccessibilityNodeObject::isIndeterminate() const
{
return supportsCheckedState() && checkboxOrRadioValue() == AccessibilityButtonState::Mixed;
if (supportsCheckedState())
return checkboxOrRadioValue() == AccessibilityButtonState::Mixed;

// We handle this for native <progress> elements in AccessibilityProgressIndicator::isIndeterminate.
if (ariaRoleAttribute() == AccessibilityRole::ProgressIndicator)
return !hasARIAValueNow();

return false;
}

bool AccessibilityNodeObject::isPressed() const
@@ -624,7 +624,7 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
bool supportsAutoComplete() const;
String autoCompleteValue() const override;

bool hasARIAValueNow() const override { return hasAttribute(HTMLNames::aria_valuenowAttr); }
bool hasARIAValueNow() const { return hasAttribute(HTMLNames::aria_valuenowAttr); }
bool supportsARIAAttributes() const;

// CSS3 Speech properties.
@@ -1290,7 +1290,6 @@ class AXCoreObject : public ThreadSafeRefCounted<AXCoreObject> {

virtual bool isValueAutofillAvailable() const = 0;
virtual AutoFillButtonType valueAutofillButtonType() const = 0;
virtual bool hasARIAValueNow() const = 0;

// Used by an ARIA tree to get all its rows.
virtual void ariaTreeRows(AccessibilityChildrenVector&) = 0;
@@ -77,6 +77,13 @@ String AccessibilityProgressIndicator::valueDescription() const
return description;
}

bool AccessibilityProgressIndicator::isIndeterminate() const
{
if (auto* progress = progressElement())
return !progress->hasAttribute(valueAttr);
return false;
}

float AccessibilityProgressIndicator::valueForRange() const
{
if (auto* progress = progressElement(); progress && progress->position() >= 0)
@@ -34,6 +34,8 @@ class AccessibilityProgressIndicator final : public AccessibilityRenderObject {
public:
static Ref<AccessibilityProgressIndicator> create(RenderObject*);

bool isIndeterminate() const final;

private:
AccessibilityRole roleValue() const override;
bool isProgressIndicator() const override { return true; }
@@ -82,7 +82,6 @@ void AXIsolatedObject::initializeProperties(const Ref<AXCoreObject>& coreObject,
else
setProperty(AXPropertyName::AncestorFlags, object.computeAncestorFlagsWithTraversal());

setProperty(AXPropertyName::HasARIAValueNow, object.hasARIAValueNow());
setProperty(AXPropertyName::IsAttachment, object.isAttachment());
setProperty(AXPropertyName::IsBusy, object.isBusy());
setProperty(AXPropertyName::IsButton, object.isButton());
@@ -92,6 +91,7 @@ void AXIsolatedObject::initializeProperties(const Ref<AXCoreObject>& coreObject,
setProperty(AXPropertyName::IsFocused, object.isFocused());
setProperty(AXPropertyName::IsGroup, object.isGroup());
setProperty(AXPropertyName::IsHeading, object.isHeading());
setProperty(AXPropertyName::IsIndeterminate, object.isIndeterminate());
setProperty(AXPropertyName::IsInlineText, object.isInlineText());
setProperty(AXPropertyName::IsInputImage, object.isInputImage());
setProperty(AXPropertyName::IsLandmark, object.isLandmark());
@@ -1499,12 +1499,6 @@ bool AXIsolatedObject::isNonNativeTextControl() const
return false;
}

bool AXIsolatedObject::isIndeterminate() const
{
ASSERT_NOT_REACHED();
return false;
}

HashMap<String, AXEditingStyleValueVariant> AXIsolatedObject::resolvedEditingStyles() const
{
return Accessibility::retrieveValueFromMainThread<HashMap<String, AXEditingStyleValueVariant>>([this] () -> HashMap<String, AXEditingStyleValueVariant> {
@@ -324,7 +324,6 @@ class AXIsolatedObject final : public AXCoreObject {
void visibleChildren(AccessibilityChildrenVector& children) override { fillChildrenVectorForProperty(AXPropertyName::VisibleChildren, children); }
void tabChildren(AccessibilityChildrenVector& children) override { fillChildrenVectorForProperty(AXPropertyName::TabChildren, children); }
AccessibilityChildrenVector contents() override;
bool hasARIAValueNow() const override { return boolAttributeValue(AXPropertyName::HasARIAValueNow); }
AtomString tagName() const override;
const AccessibilityChildrenVector& children(bool updateChildrenIfNeeded = true) override;
void updateChildrenIfNecessary() override;
@@ -457,7 +456,7 @@ class AXIsolatedObject final : public AXCoreObject {
bool isImageMapLink() const override;
bool isMockObject() const override;
bool isNonNativeTextControl() const override;
bool isIndeterminate() const override;
bool isIndeterminate() const override { return boolAttributeValue(AXPropertyName::IsIndeterminate); }
bool isLoaded() const override { return loadingProgress() >= 1; }
bool isOnScreen() const override;
bool isOffScreen() const override;
@@ -102,7 +102,6 @@ enum class AXPropertyName : uint16_t {
EmbeddedImageDescription,
ExpandedTextValue,
ExtendedDescription,
HasARIAValueNow,
HasApplePDFAnnotationAttribute,
HasBoldFont,
HasHighlighting,
@@ -132,6 +131,7 @@ enum class AXPropertyName : uint16_t {
IsFieldset,
IsFocused,
IsGroup,
IsIndeterminate,
IsInlineText,
IsInputImage,
IsInsideLiveRegion,
@@ -2130,14 +2130,14 @@ - (id)accessibilityAttributeValue:(NSString*)attributeName

if ([attributeName isEqualToString: NSAccessibilityMinValueAttribute]) {
// Indeterminate progress indicator should return 0.
if (backingObject->ariaRoleAttribute() == AccessibilityRole::ProgressIndicator && !backingObject->hasARIAValueNow())
if (backingObject->isIndeterminate())
return @0;
return [NSNumber numberWithFloat:backingObject->minValueForRange()];
}

if ([attributeName isEqualToString: NSAccessibilityMaxValueAttribute]) {
// Indeterminate progress indicator should return 0.
if (backingObject->ariaRoleAttribute() == AccessibilityRole::ProgressIndicator && !backingObject->hasARIAValueNow())
if (backingObject->isIndeterminate())
return @0;
return [NSNumber numberWithFloat:backingObject->maxValueForRange()];
}
@@ -2669,6 +2669,9 @@ - (id)accessibilityAttributeValue:(NSString*)attributeName
if ([attributeName isEqualToString:@"AXIsOnScreen"])
return [NSNumber numberWithBool:backingObject->isOnScreen()];

if ([attributeName isEqualToString:@"AXIsIndeterminate"])
return [NSNumber numberWithBool: backingObject->isIndeterminate()];

return nil;
}

@@ -1064,13 +1064,7 @@ void setAttributeValue(id element, NSString* attribute, id value, bool synchrono

bool AccessibilityUIElement::isIndeterminate() const
{
BEGIN_AX_OBJC_EXCEPTIONS
auto value = attributeValue(NSAccessibilityValueAttribute);
if ([value isKindOfClass:[NSNumber class]])
return [value intValue] == 2;
END_AX_OBJC_EXCEPTIONS

return false;
return boolAttributeValue(@"AXIsIndeterminate");
}

bool AccessibilityUIElement::isExpanded() const

0 comments on commit ece8b0e

Please sign in to comment.