Bug 260190 – AX: aria-labelledby-on-password-input fails in ITM

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Joshua Hoffman]

In isolated tree mode, accessibility/aria-labelledby-on-password-input.html fails due to the linked nodes not updating properly.

[Radar WebKit Bug Importer]

<rdar://problem/113885331>

[Joshua Hoffman]

Created attachment 467283 [details]
Patch

[Andres Gonzalez]

(In reply to Joshua Hoffman from comment #2)
> Created attachment 467283 [details]
> Patch

--- a/Source/WebCore/accessibility/AXObjectCache.cpp
+++ b/Source/WebCore/accessibility/AXObjectCache.cpp
@@ -1282,6 +1282,11 @@ void AXObjectCache::handleChildrenChanged(AccessibilityObject& object)
             // Do not let any ancestor of an editable object update its children.
             shouldUpdateParent = false;
         }
+
+        if (auto objects = parent->labelForObjects(); objects.size()) {
+            for (const auto& axObject : objects)
+                postNotification(dynamicDowncast<AccessibilityObject>(axObject.get()), axObject->document(), AXValueChanged);
+        }

handleChildrenChanged doesn't seem the right place to add this notification. Look at

AXObjectCache::handleTextChanged
AXObjectCache::valueChanged


--- a/LayoutTests/accessibility/aria-labelledby-on-password-input.html
+++ b/LayoutTests/accessibility/aria-labelledby-on-password-input.html

-        await waitFor(() => password2.stringValue === "AXValue: \u2022\u2022\u2022\u2022\u2022");
+        await waitFor(() => {
+            return password2.stringValue === "AXValue: \u2022\u2022\u2022\u2022\u2022"
+            && button2.description === "AXDescription: Before password \u2022\u2022\u2022\u2022\u2022 After password";

Indent line starting with &&.

[Joshua Hoffman]

Created attachment 467314 [details]
Patch

[Joshua Hoffman]

(In reply to Andres Gonzalez from comment #3)
> (In reply to Joshua Hoffman from comment #2)
> > Created attachment 467283 [details]
> > Patch
            
> postNotification(dynamicDowncast<AccessibilityObject>(axObject.get()),
> axObject->document(), AXValueChanged);
> +        }
> 
> handleChildrenChanged doesn't seem the right place to add this notification.
> Look at
> 

Placing this codepath inside handleChildrenChanged allows us to update LabelledBy elements regardless of what the change is (since it won't always just be a text change). I collected samples as well and this addition doesn't have an impact on the cost of the method. 

The new test added in the latest patch tests for text changes, to add on to the value change we have covered in the password-input test.

[Joshua Hoffman]

Created attachment 467316 [details]
Patch

[Andres Gonzalez]

(In reply to Joshua Hoffman from comment #6)
> Created attachment 467316 [details]
> Patch

diff --git a/LayoutTests/accessibility/aria-labelledby-text.html b/LayoutTests/accessibility/aria-labelledby-text.html
new file mode 100644
index 000000000000..d453dbdf468b
--- /dev/null
+++ b/LayoutTests/accessibility/aria-labelledby-text.html

+    output += expect("button1.description", "'AXDescription: Label for Button One'");
+    output += expect("button2.description", "'AXDescription: '");
+
+    setTimeout(async function() {
+        await waitFor(() => button1.description === "AXDescription: Label for Button One");
+        output += expect("button1.description", "'AXDescription: Label for Button One'");

No need to repeat this check here since it was already checked in the first expect() above and nothing has changed.

[Joshua Hoffman]

Created attachment 467329 [details]
Patch

[EWS]

Committed 267086@main (d15d7af722dc): <https://commits.webkit.org/267086@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 467329 [details].