Skip to content

Commit

Permalink
AX: Content editable should not be textbox roles
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=257184
rdar://problem/109699222

Reviewed by Chris Fleizach.

We currently expose elements with contenteditable=true as AXTextAreas. However, text areas
cannot have children objects that are exposed to AT (such as tables, lists, etc.).

This patch addresses that by changing the role for contenteditable (which are non-native text
control elements) elements to AXGroup, allowing for its children to be navigated. We still
maintain the edit-ability of these elements however by exposing AXValue as editable.

On iOS, we use traits to expose leaf elements with a non-native text control ancestor as editable
for AT to supply proper context.

Tests have been updated and added to reflect this change.

This patch was authored by Josh Hoffman, built on work started by Chris Fleizach.

* LayoutTests/accessibility/content-editable-property-change-expected.txt: Added.
* LayoutTests/accessibility/content-editable-property-change.html: Added.
* LayoutTests/accessibility/dynamically-unignored-contenteditable-expected.txt:
* LayoutTests/accessibility/ios-simulator/content-editable-expected.txt: Added.
* LayoutTests/accessibility/ios-simulator/content-editable.html: Added.
* LayoutTests/accessibility/mac/active-descendant-with-aria-controls-expected.txt:
* LayoutTests/accessibility/mac/relationships-in-frames-expected.txt:
* LayoutTests/accessibility/mac/set-value-editable-dispatch-events-expected.txt:
* LayoutTests/accessibility/mac/set-value-editable-types-expected.txt:
* LayoutTests/accessibility/nested-textareas-value-changed-notifications-expected.txt:
* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/glib/accessibility/content-editable-as-textarea-expected.txt:
* LayoutTests/platform/glib/accessibility/dynamically-unignored-contenteditable-expected.txt:
* LayoutTests/platform/mac-wk1/TestExpectations:
* LayoutTests/platform/mac/accessibility/content-editable-as-textarea-expected.txt:

* Source/WebCore/accessibility/AXCoreObject.cpp:
(WebCore::AXCoreObject::isTextControl const):
* Source/WebCore/accessibility/AXLogger.cpp:
(WebCore::operator<<):
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::handleAttributeChange):
(WebCore::AXObjectCache::updateIsolatedTree):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::matchesTextAreaRole const):
(WebCore::AccessibilityNodeObject::determineAccessibilityRoleFromNode const):
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::isNonNativeTextControl const):
* Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:
(-[WebAccessibilityObjectWrapper accessibilityTraits]):
(-[WebAccessibilityObjectWrapper determineIsAccessibilityElement]):
(-[WebAccessibilityObjectWrapper accessibilityIsInNonNativeTextControl]):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::initializeProperties):
(WebCore::AXIsolatedObject::isNonNativeTextControl const): Deleted.
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::updateNodeProperties):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:

* Tools/DumpRenderTree/AccessibilityUIElement.cpp:
(getIsInNonNativeTextControlCallback):
(AccessibilityUIElement::getJSClass):
* Tools/DumpRenderTree/AccessibilityUIElement.h:
* Tools/DumpRenderTree/ios/AccessibilityUIElementIOS.mm:
(AccessibilityUIElement::isInNonNativeTextControl const):
* Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h:
* Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl:
* Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityUIElementAtspi.cpp:
(WTR::AccessibilityUIElement::isInNonNativeTextControl const):
* Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm:
(WTR::AccessibilityUIElement::isInNonNativeTextControl const):
* Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::isInNonNativeTextControl const):
* Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityUIElementWin.cpp:
(WTR::AccessibilityUIElement::isInNonNativeTextControl const):

Canonical link: https://commits.webkit.org/270935@main
  • Loading branch information
twilco committed Nov 18, 2023
1 parent 14d4ec3 commit 0524e64
Show file tree
Hide file tree
Showing 33 changed files with 179 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
This test ensures that changes to the contenteditable property are reflected in accessibility

PASS: button.editableAncestor() === null
PASS: button.editableAncestor().domIdentifier === 'content'

PASS successfullyParsed is true

TEST COMPLETE
Press Me
28 changes: 28 additions & 0 deletions LayoutTests/accessibility/content-editable-property-change.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<html>
<head>
<script src="../resources/accessibility-helper.js"></script>
<script src="../resources/js-test.js"></script>
</head>
<body>

<div id="content">
<button id="button">Press Me</button>
</div>

<script>
let output = "This test ensures that changes to the contenteditable property are reflected in accessibility\n\n";
if (window.accessibilityController) {
window.jsTestIsAsync = true;
var button = accessibilityController.accessibleElementById("button");
output += expect("button.editableAncestor()", "null");

document.getElementById("content").contentEditable = "true";
setTimeout(async () => {
await waitFor(() => button.editableAncestor());
output += expect("button.editableAncestor().domIdentifier", "'content'");
debug(output);
finishJSTest();
});
}
</script>
</body>
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ This test ensures that we properly change an object's ignored status after dynam
Adding contenteditable='true' to #div.
#div is ignored: false
#wrapper children:
AXRole: AXTextArea
AXRole: AXGroup

Removing contenteditable='true' from #div.
#div is ignored: true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
This test makes sure that the content editable method on iOS works as expected.
PASS: text1.isInNonNativeTextControl === true
PASS: text2.isInNonNativeTextControl === true
PASS: text3.isInNonNativeTextControl === false

PASS successfullyParsed is true

TEST COMPLETE
Text inside

Nested text inside

Text outside
35 changes: 35 additions & 0 deletions LayoutTests/accessibility/ios-simulator/content-editable.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
<head>
<script src="../../resources/js-test.js"></script>
<script src="../../resources/accessibility-helper.js"></script>
</head>
<body>

<div id="editable1" contenteditable="true">
<p id="text1">Text inside</p>
<div>
<p id="text2">Nested text inside</p>
</div>
</div>

<p id="text3">Text outside</p>

<script>
var output = "This test makes sure that the content editable method on iOS works as expected.\n";

if (window.accessibilityController) {
var text1 = accessibilityController.accessibleElementById("text1").childAtIndex(0);
output += expect("text1.isInNonNativeTextControl", "true");

var text2 = accessibilityController.accessibleElementById("text2").childAtIndex(0);
output += expect("text2.isInNonNativeTextControl", "true");

var text3 = accessibilityController.accessibleElementById("text3").childAtIndex(0);
output += expect("text3.isInNonNativeTextControl", "false");

debug(output);
}
</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
This test ensures objects that control objects with active descendants generate the correct notifications.

addedNotification: true
Controller role: AXRole: AXTextArea
Controller role: AXRole: AXTextField
Received selected children notification AXRole: AXList
PASS successfullyParsed is true

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
controller role: AXRole: AXTextArea
controller role: AXRole: AXTextField
controller controls role: listbox
listbox role: AXRole: AXList
aria-controls textbox->listbox : true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE


Test: contenteditable
Role: AXRole: AXTextArea
Role: AXRole: AXGroup
Value: AXValue: current
Writable: true
input event dispatched for contenteditable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE


Contenteditable
Role: AXRole: AXTextArea
Role: AXRole: AXGroup
Value: AXValue: current1
Writable: true
Value change notification received
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,25 +4,25 @@ xyz
def
1 AXValueChanged for element with AXRole: AXWebArea
AXValue:
2 AXValueChanged for element outer_editable with AXRole: AXTextArea
2 AXValueChanged for element outer_editable with AXRole: AXGroup
AXValue: aabc
xyz
def
3 AXValueChanged for element with AXRole: AXWebArea
AXValue:
4 AXValueChanged for element outer_editable with AXRole: AXTextArea
4 AXValueChanged for element outer_editable with AXRole: AXGroup
AXValue: aabc
xyz
def
5 AXValueChanged for element with AXRole: AXWebArea
AXValue:
6 AXValueChanged for element outer_editable with AXRole: AXTextArea
6 AXValueChanged for element outer_editable with AXRole: AXGroup
AXValue: aabc
xyz
dgef
7 AXValueChanged for element inner_editable with AXRole: AXTextArea
7 AXValueChanged for element inner_editable with AXRole: AXGroup
AXValue: dgef
8 AXValueChanged for element inner_editable with AXRole: AXTextArea
8 AXValueChanged for element inner_editable with AXRole: AXGroup
AXValue: dgef

PASS successfullyParsed is true
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ accessibility/focusable-ancestor.html [ Skip ]

# Missing AccessibilityUIElement::{editableAncestor, highestEditableAncestor} implementations.
accessibility/editable-ancestor.html [ Skip ]
accessibility/content-editable-property-change.html [ Skip ]

# Timing out since it was added in https://bugs.webkit.org/show_bug.cgi?id=239434.
accessibility/text-updates-after-dynamic-change.html [ Skip ]
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/platform/mac-wk1/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,7 @@ accessibility/focusable-ancestor.html [ Skip ]

# Missing AccessibilityUIElement::{editableAncestor, highestEditableAncestor} implementations.
accessibility/editable-ancestor.html [ Skip ]
accessibility/content-editable-property-change.html [ Skip ]

# <rdar://problem/26050923> The result is probably still a pass, but we don't have a way
# to have platform specific results that are different between WK1 and WK2.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
This tests that a contenteditable region behaves as a proper AXTextArea.

Role: AXRole: AXTextArea
Role: AXRole: AXGroup
Value: AXValue: hello
world
Value (writable): true
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/accessibility/AXCoreObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ bool AXCoreObject::isButton() const

bool AXCoreObject::isTextControl() const
{
if (isNonNativeTextControl())
return true;

switch (roleValue()) {
case AccessibilityRole::ComboBox:
case AccessibilityRole::SearchField:
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/accessibility/AXLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,9 @@ TextStream& operator<<(TextStream& stream, AXObjectCache::AXNotification notific
case AXObjectCache::AXNotification::AXColumnSpanChanged:
stream << "AXColumnSpanChanged";
break;
case AXObjectCache::AXNotification::AXContentEditableAttributeChanged:
stream << "AXContentEditableAttributeChanged";
break;
case AXObjectCache::AXNotification::AXControlledObjectsChanged:
stream << "AXControlledObjectsChanged";
break;
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/accessibility/AXObjectCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2428,6 +2428,7 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName&
else if (attrName == contenteditableAttr) {
if (auto* axObject = get(element))
axObject->updateRole();
postNotification(element, AXContentEditableAttributeChanged);
}
else if (attrName == disabledAttr)
postNotification(element, AXDisabledStateChanged);
Expand Down Expand Up @@ -4161,6 +4162,9 @@ void AXObjectCache::updateIsolatedTree(const Vector<std::pair<RefPtr<Accessibili
case AXColumnSpanChanged:
tree->updateNodeProperty(*notification.first, AXPropertyName::ColumnIndexRange);
break;
case AXContentEditableAttributeChanged:
tree->updateNodeProperty(*notification.first, AXPropertyName::IsNonNativeTextControl);
break;
case AXDisabledStateChanged:
tree->updatePropertiesForSelfAndDescendants(*notification.first, { AXPropertyName::CanSetFocusAttribute, AXPropertyName::IsEnabled });
break;
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/accessibility/AXObjectCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
AXColumnCountChanged,
AXColumnIndexChanged,
AXColumnSpanChanged,
AXContentEditableAttributeChanged,
AXControlledObjectsChanged,
AXCurrentStateChanged,
AXDescribedByChanged,
Expand Down
13 changes: 11 additions & 2 deletions Source/WebCore/accessibility/AccessibilityNodeObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,11 @@ AccessibilityRole AccessibilityNodeObject::determineAccessibilityRole()

bool AccessibilityNodeObject::matchesTextAreaRole() const
{
return is<HTMLTextAreaElement>(node()) || hasContentEditableAttributeSet();
#if !PLATFORM(COCOA)
if (hasContentEditableAttributeSet())
return true;
#endif
return is<HTMLTextAreaElement>(node());
}

AccessibilityRole AccessibilityNodeObject::determineAccessibilityRoleFromNode(TreatStyleFormatGroupAsInline treatStyleFormatGroupAsInline) const
Expand Down Expand Up @@ -430,7 +434,7 @@ AccessibilityRole AccessibilityNodeObject::determineAccessibilityRoleFromNode(Tr
return AccessibilityRole::Label;
if (node()->hasTagName(dfnTag))
return AccessibilityRole::Definition;
if (node()->hasTagName(divTag))
if (node()->hasTagName(divTag) && !isNonNativeTextControl())
return AccessibilityRole::Generic;
if (is<HTMLFormElement>(node()))
return AccessibilityRole::Form;
Expand Down Expand Up @@ -479,6 +483,11 @@ AccessibilityRole AccessibilityNodeObject::determineAccessibilityRoleFromNode(Tr
if (auto* summaryElement = dynamicDowncast<HTMLSummaryElement>(node()); summaryElement && summaryElement->isActiveSummary())
return AccessibilityRole::Summary;

#if PLATFORM(COCOA)
if (isNonNativeTextControl())
return AccessibilityRole::Group;
#endif

// http://rawgit.com/w3c/aria/master/html-aam/html-aam.html
// Output elements should be mapped to status role.
if (isOutput())
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ bool AccessibilityObject::isARIATextControl() const

bool AccessibilityObject::isNonNativeTextControl() const
{
return (isARIATextControl() || hasContentEditableAttributeSet()) && !isNativeTextControl();
return (hasContentEditableAttributeSet() || isARIATextControl()) && !isNativeTextControl();
}

bool AccessibilityObject::hasMisspelling() const
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,9 @@ - (uint64_t)accessibilityTraits
break;
}

if ([self accessibilityIsInNonNativeTextControl])
traits |= [self _accessibilityTextEntryTraits];

if (self.axBackingObject->isAttachmentElement())
traits |= [self _axUpdatesFrequentlyTrait];

Expand Down Expand Up @@ -995,6 +998,9 @@ - (BOOL)determineIsAccessibilityElement
case AccessibilityRole::Video:
return [self accessibilityIsWebInteractiveVideo];

if (self.axBackingObject->isNonNativeTextControl())
return true;

// Links can sometimes be elements (when they only contain static text or don't contain anything).
// They should not be elements when containing text and other types.
case AccessibilityRole::WebCoreLink:
Expand Down Expand Up @@ -2750,6 +2756,13 @@ - (BOOL)accessibilityIsDeletion
}) != nullptr;
}

- (BOOL)accessibilityIsInNonNativeTextControl
{
return Accessibility::findAncestor(*self.axBackingObject, true, [] (const auto& object) {
return object.isNonNativeTextControl();
});
}

- (BOOL)accessibilityIsFirstItemInSuggestion
{
if (![self _prepareAccessibilityCall])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb
setProperty(AXPropertyName::IsKeyboardFocusable, object.isKeyboardFocusable());
setProperty(AXPropertyName::BrailleRoleDescription, object.brailleRoleDescription().isolatedCopy());
setProperty(AXPropertyName::BrailleLabel, object.brailleLabel().isolatedCopy());
setProperty(AXPropertyName::IsNonNativeTextControl, object.isNonNativeTextControl());

RefPtr geometryManager = tree()->geometryManager();
std::optional frame = geometryManager ? geometryManager->cachedRectForID(object.objectID()) : std::nullopt;
Expand Down Expand Up @@ -1522,12 +1523,6 @@ bool AXIsolatedObject::isMockObject() const
return false;
}

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

HashMap<String, AXEditingStyleValueVariant> AXIsolatedObject::resolvedEditingStyles() const
{
return Accessibility::retrieveValueFromMainThread<HashMap<String, AXEditingStyleValueVariant>>([this] () -> HashMap<String, AXEditingStyleValueVariant> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ class AXIsolatedObject final : public AXCoreObject {
bool isNativeTextControl() const final;
bool isListBoxOption() const final;
bool isMockObject() const final;
bool isNonNativeTextControl() const final;
bool isNonNativeTextControl() const final { return boolAttributeValue(AXPropertyName::IsNonNativeTextControl); }
bool isIndeterminate() const final { return boolAttributeValue(AXPropertyName::IsIndeterminate); }
bool isLoaded() const final { return loadingProgress() >= 1; }
bool isOnScreen() const final;
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,9 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const Vector<A
case AXPropertyName::AXColumnIndex:
propertyMap.set(AXPropertyName::AXColumnIndex, axObject.axColumnIndex());
break;
case AXPropertyName::IsNonNativeTextControl:
propertyMap.set(AXPropertyName::IsNonNativeTextControl, axObject.isNonNativeTextControl());
break;
case AXPropertyName::CanSetFocusAttribute:
propertyMap.set(AXPropertyName::CanSetFocusAttribute, axObject.canSetFocusAttribute());
break;
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ enum class AXPropertyName : uint16_t {
IsMathToken,
IsMeter,
IsMultiSelectable,
IsNonNativeTextControl,
IsPlugin,
IsPressed,
IsRequired,
Expand Down
6 changes: 6 additions & 0 deletions Tools/DumpRenderTree/AccessibilityUIElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1675,6 +1675,11 @@ static JSValueRef getIsMarkAnnotationCallback(JSContextRef context, JSObjectRef
return JSValueMakeBoolean(context, toAXElement(thisObject)->isMarkAnnotation());
}

static JSValueRef getIsInNonNativeTextControlCallback(JSContextRef context, JSObjectRef thisObject, JSStringRef propertyName, JSValueRef* exception)
{
return JSValueMakeBoolean(context, toAXElement(thisObject)->isInNonNativeTextControl());
}

#endif // PLATFORM(IOS_FAMILY)

#if PLATFORM(MAC) && !PLATFORM(IOS_FAMILY)
Expand Down Expand Up @@ -2050,6 +2055,7 @@ JSClassRef AccessibilityUIElement::getJSClass()
{ "isFirstItemInSuggesiton", getIsFirstItemInSuggestionCallback, 0 , kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
{ "isLastItemInSuggesiton", getIsLastItemInSuggestionCallback, 0 , kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
{ "isMarkAnnotation", getIsMarkAnnotationCallback, 0 , kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
{ "isInNonNativeTextControl", getIsInNonNativeTextControlCallback, 0 , kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
#endif // PLATFORM(IOS_FAMILY)
#if PLATFORM(MAC) && !PLATFORM(IOS_FAMILY)
{ "supportedActions", supportedActionsCallback, 0, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
Expand Down
2 changes: 2 additions & 0 deletions Tools/DumpRenderTree/AccessibilityUIElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ class AccessibilityUIElement {
bool isInsertion();
bool isFirstItemInSuggestion();
bool isLastItemInSuggestion();

bool isInNonNativeTextControl() const;
#endif

// Table-specific
Expand Down
Loading

0 comments on commit 0524e64

Please sign in to comment.