| Summary: | AX: aria-labelledby-on-password-input fails in ITM | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Joshua Hoffman <jhoffman23> | ||||||||||
| Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Joshua Hoffman
2023-08-14 19:27:49 PDT
Created attachment 467283 [details]
Patch
(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 &&. Created attachment 467314 [details]
Patch
(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. Created attachment 467316 [details]
Patch
(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. Created attachment 467329 [details]
Patch
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]. |