From 4ef6169e54ce65e7b1a213c7917a9ee69baf0e75 Mon Sep 17 00:00:00 2001 From: Andres Gonzalez Date: Fri, 12 Jan 2024 17:13:16 -0800 Subject: [PATCH] AX: Refactor the TitleUIElement property using LabeledBy and LabelFor relationships. https://bugs.webkit.org/show_bug.cgi?id=262787 Reviewed by Tyler Wilcock. The TitleUIElement property was treated as being independent from the object's LabeledBy and LabelFor relationships. Conceptually, TitleUIElement and LabeledBy are the same concept, and indeed shared a lot of common code in the AccessibilityObject class hierarchy. This change unifies the implementation of TitleUIElement and the Label relationships in a coherent way. With this change is no longer necessary to cache and update the TitleUIElement in the IsolatedObject as a separate property, but instead it is stored as a relationship. This change will also allow the addition of enhanced heuristics to determine label-object associations. Some code and style cleanup is also part of the patch. Canonical link: https://commits.webkit.org/273000@main --- ...-hidden-negates-no-visibility-expected.txt | 2 +- ...-labelledby-on-password-input-expected.txt | 8 +- .../aria-labelledby-on-password-input.html | 11 +- .../aria-labelledby-text-expected.txt | 10 +- .../accessibility/aria-labelledby-text.html | 12 +- ...-label-overrides-visible-text-expected.txt | 2 +- .../aria-label-overrides-visible-text.html | 2 +- .../radio-button-title-label-expected.txt | 4 +- .../radio-button-title-label.html | 4 +- .../accessibility/th-as-title-ui-expected.txt | 3 - LayoutTests/accessibility/th-as-title-ui.html | 42 -- LayoutTests/platform/glib/TestExpectations | 1 - ...belledby-overrides-aria-label-expected.txt | 6 +- .../aria-switch-text-expected.txt | 2 +- .../w3c-svg-name-calculation-expected.txt | 12 +- Source/WebCore/accessibility/AXCoreObject.cpp | 12 + Source/WebCore/accessibility/AXCoreObject.h | 8 +- Source/WebCore/accessibility/AXLogger.cpp | 10 +- .../WebCore/accessibility/AXObjectCache.cpp | 225 +++++++---- Source/WebCore/accessibility/AXObjectCache.h | 10 +- .../accessibility/AccessibilityLabel.cpp | 27 -- .../accessibility/AccessibilityLabel.h | 8 +- .../accessibility/AccessibilityNodeObject.cpp | 368 +++++++++--------- .../accessibility/AccessibilityNodeObject.h | 19 +- .../accessibility/AccessibilityObject.cpp | 9 +- .../accessibility/AccessibilityObject.h | 7 +- .../AccessibilityProgressIndicator.cpp | 2 +- .../AccessibilityRenderObject.cpp | 15 +- .../accessibility/AccessibilityRenderObject.h | 4 +- .../accessibility/AccessibilityTable.cpp | 2 +- .../accessibility/AccessibilityTable.h | 2 +- .../atspi/AccessibilityObjectAtspi.cpp | 20 +- .../ios/WebAccessibilityObjectWrapperIOS.mm | 17 +- .../isolatedtree/AXIsolatedObject.cpp | 13 - .../isolatedtree/AXIsolatedObject.h | 3 - .../isolatedtree/AXIsolatedTree.cpp | 40 +- .../isolatedtree/AXIsolatedTree.h | 4 +- .../mac/WebAccessibilityObjectWrapperMac.mm | 3 +- 38 files changed, 484 insertions(+), 465 deletions(-) delete mode 100644 LayoutTests/accessibility/th-as-title-ui-expected.txt delete mode 100644 LayoutTests/accessibility/th-as-title-ui.html diff --git a/LayoutTests/accessibility/aria-hidden-negates-no-visibility-expected.txt b/LayoutTests/accessibility/aria-hidden-negates-no-visibility-expected.txt index 3ef5f69dc930..fb76acc041e4 100644 --- a/LayoutTests/accessibility/aria-hidden-negates-no-visibility-expected.txt +++ b/LayoutTests/accessibility/aria-hidden-negates-no-visibility-expected.txt @@ -16,7 +16,7 @@ PASS heading3.role is 'AXRole: AXHeading' PASS parent.childAtIndex(1).isEqual(heading3) is true PASS heading4.role is 'AXRole: AXHeading' PASS parent.childAtIndex(2).isEqual(heading4) is true -Textfield Title: AXTitle: +Textfield Title: AXTitle: HiddenText1 PASS !video || video.childrenCount == 0 is true PASS successfullyParsed is true diff --git a/LayoutTests/accessibility/aria-labelledby-on-password-input-expected.txt b/LayoutTests/accessibility/aria-labelledby-on-password-input-expected.txt index b92f2e7dcc11..3bc2469f7a96 100644 --- a/LayoutTests/accessibility/aria-labelledby-on-password-input-expected.txt +++ b/LayoutTests/accessibility/aria-labelledby-on-password-input-expected.txt @@ -1,15 +1,15 @@ This test ensures a label from aria-labelledby does not include a password inputs value. PASS: button1.description === 'AXDescription: ' -PASS: button1.title === 'AXTitle: Button One' -PASS: button1.description === 'AXDescription: •••••' PASS: button1.title === 'AXTitle: ' +PASS: button1.description === 'AXDescription: •••••' +PASS: button1.title === 'AXTitle: •••••' Update aria-labelledby to text-1 for #button-2 PASS: button2.description === 'AXDescription: Before password After password' -PASS: button2.title === 'AXTitle: ' +PASS: button2.title === 'AXTitle: Before password After password' Update value for #password-2 PASS: button2.description === 'AXDescription: Before password ••••• After password' -PASS: button2.title === 'AXTitle: ' +PASS: button2.title === 'AXTitle: Before password ••••• After password' PASS successfullyParsed is true diff --git a/LayoutTests/accessibility/aria-labelledby-on-password-input.html b/LayoutTests/accessibility/aria-labelledby-on-password-input.html index 8a3dd2220e85..62cd47a05205 100644 --- a/LayoutTests/accessibility/aria-labelledby-on-password-input.html +++ b/LayoutTests/accessibility/aria-labelledby-on-password-input.html @@ -5,6 +5,7 @@ +
@@ -22,19 +23,19 @@ var password1 = accessibilityController.accessibleElementById("password-1"); var password2 = accessibilityController.accessibleElementById("password-2"); output += expect("button1.description", "'AXDescription: '"); - output += expect("button1.title", "'AXTitle: Button One'"); + output += expect("button1.title", "'AXTitle: '"); document.getElementById("password-1").value = "hello"; setTimeout(async function() { await waitFor(() => password1.stringValue === "AXValue: \u2022\u2022\u2022\u2022\u2022"); output += expect("button1.description", "'AXDescription: \u2022\u2022\u2022\u2022\u2022'"); - output += expect("button1.title", "'AXTitle: '"); + output += expect("button1.title", "'AXTitle: \u2022\u2022\u2022\u2022\u2022'"); output += "Update aria-labelledby to text-1 for #button-2\n"; document.getElementById("button-2").setAttribute("aria-labelledby", "text-1"); await waitFor(() => button2.description === "AXDescription: Before password After password"); output += expect("button2.description", "'AXDescription: Before password After password'"); - output += expect("button2.title", "'AXTitle: '"); + output += expect("button2.title", "'AXTitle: Before password After password'"); output += "Update value for #password-2\n"; document.getElementById("password-2").value = "hello"; @@ -43,7 +44,7 @@ && button2.description === "AXDescription: Before password \u2022\u2022\u2022\u2022\u2022 After password"; }); output += expect("button2.description", "'AXDescription: Before password \u2022\u2022\u2022\u2022\u2022 After password'"); - output += expect("button2.title", "'AXTitle: '"); + output += expect("button2.title", "'AXTitle: Before password \u2022\u2022\u2022\u2022\u2022 After password'"); debug(output); finishJSTest(); @@ -51,4 +52,4 @@ } - \ No newline at end of file + diff --git a/LayoutTests/accessibility/aria-labelledby-text-expected.txt b/LayoutTests/accessibility/aria-labelledby-text-expected.txt index 186f126b4fa3..a8d053a3db52 100644 --- a/LayoutTests/accessibility/aria-labelledby-text-expected.txt +++ b/LayoutTests/accessibility/aria-labelledby-text-expected.txt @@ -1,17 +1,15 @@ This test ensures that aria-labelledby elements return the right AXDescription when their labels are updated. PASS: button1.description === 'AXDescription: Label for Button One' -PASS: button1.title === 'AXTitle: ' +PASS: button1.title === 'AXTitle: Label for Button One' PASS: button2.description === 'AXDescription: ' PASS: button2.description === 'AXDescription: Label for Button Two' -PASS: button2.title === 'AXTitle: ' +PASS: button2.title === 'AXTitle: Label for Button Two' PASS: button1.description === 'AXDescription: Label for Button One!' -PASS: button1.title === 'AXTitle: ' +PASS: button1.title === 'AXTitle: Label for Button One!' PASS successfullyParsed is true TEST COMPLETE -Button One Button Two -Label for Button One! -Label for Button Two + diff --git a/LayoutTests/accessibility/aria-labelledby-text.html b/LayoutTests/accessibility/aria-labelledby-text.html index 8663d1579912..2f92a4d7613e 100644 --- a/LayoutTests/accessibility/aria-labelledby-text.html +++ b/LayoutTests/accessibility/aria-labelledby-text.html @@ -5,7 +5,8 @@ -
+ +

Label for Button One

@@ -23,24 +24,25 @@ var label2 = accessibilityController.accessibleElementById("label2"); output += expect("button1.description", "'AXDescription: Label for Button One'"); - output += expect("button1.title", "'AXTitle: '"); + output += expect("button1.title", "'AXTitle: Label for Button One'"); output += expect("button2.description", "'AXDescription: '"); setTimeout(async function() { document.getElementById("button-2").setAttribute("aria-labelledby", "label2"); await waitFor(() => button2.description === "AXDescription: Label for Button Two"); output += expect("button2.description", "'AXDescription: Label for Button Two'"); - output += expect("button2.title", "'AXTitle: '"); + output += expect("button2.title", "'AXTitle: Label for Button Two'"); document.getElementById("label1").innerText += "!"; await waitFor(() => button1.description === "AXDescription: Label for Button One!"); output += expect("button1.description", "'AXDescription: Label for Button One!'"); - output += expect("button1.title", "'AXTitle: '"); + output += expect("button1.title", "'AXTitle: Label for Button One!'"); + document.getElementById("content").style.visibility = "hidden"; debug(output); finishJSTest(); }, 0); } - \ No newline at end of file + diff --git a/LayoutTests/accessibility/mac/aria-label-overrides-visible-text-expected.txt b/LayoutTests/accessibility/mac/aria-label-overrides-visible-text-expected.txt index 2699d0733f13..d7d56fda103d 100644 --- a/LayoutTests/accessibility/mac/aria-label-overrides-visible-text-expected.txt +++ b/LayoutTests/accessibility/mac/aria-label-overrides-visible-text-expected.txt @@ -2,7 +2,7 @@ This tests ensures that if aria-label is used, the AXTitle will not expose its c PASS: link1.description === 'AXDescription: ' PASS: link1.title === 'AXTitle: test1' PASS: link2.description === 'AXDescription: LINK' -PASS: link2.title === 'AXTitle: ' +PASS: link2.title === 'AXTitle: LINK' PASS successfullyParsed is true diff --git a/LayoutTests/accessibility/mac/aria-label-overrides-visible-text.html b/LayoutTests/accessibility/mac/aria-label-overrides-visible-text.html index ec1cf8b2d544..c4311d0b4ada 100644 --- a/LayoutTests/accessibility/mac/aria-label-overrides-visible-text.html +++ b/LayoutTests/accessibility/mac/aria-label-overrides-visible-text.html @@ -21,7 +21,7 @@ var link2 = accessibilityController.accessibleElementById("link2"); output += expect("link2.description", "'AXDescription: LINK'"); - output += expect("link2.title", "'AXTitle: '"); + output += expect("link2.title", "'AXTitle: LINK'"); debug(output); document.getElementById("content").style.visibility = "hidden"; diff --git a/LayoutTests/accessibility/radio-button-title-label-expected.txt b/LayoutTests/accessibility/radio-button-title-label-expected.txt index 8aa9124828e3..cc02f2ded217 100644 --- a/LayoutTests/accessibility/radio-button-title-label-expected.txt +++ b/LayoutTests/accessibility/radio-button-title-label-expected.txt @@ -3,10 +3,10 @@ This test checks that radio buttons expose title ui elements correctly under a v PASS: radio1.title === 'AXTitle: LABEL' PASS: titleUIElement.isEqual(accessibilityController.accessibleElementById('label1')) === true PASS: radio2.description === 'AXDescription: LABEL2a' -PASS: radio2.title === 'AXTitle: ' +PASS: radio2.title === 'AXTitle: LABEL2a' PASS: !titleUIElement || titleUIElement.title == 'AXTitle: ' === true PASS: radio3.description === 'AXDescription: radio3' -PASS: radio3.title === 'AXTitle: ' +PASS: radio3.title === 'AXTitle: radio3' PASS: !titleUIElement || titleUIElement.title == 'AXTitle: ' === true PASS successfullyParsed is true diff --git a/LayoutTests/accessibility/radio-button-title-label.html b/LayoutTests/accessibility/radio-button-title-label.html index 2f9f5fd16631..ef1699bd7bb7 100644 --- a/LayoutTests/accessibility/radio-button-title-label.html +++ b/LayoutTests/accessibility/radio-button-title-label.html @@ -38,7 +38,7 @@ output += expect("radio2.description", "'AXDescription: '"); } else { output += expect("radio2.description", "'AXDescription: LABEL2a'"); - output += expect("radio2.title", "'AXTitle: '"); + output += expect("radio2.title", "'AXTitle: LABEL2a'"); } output += expect("!titleUIElement || titleUIElement.title == 'AXTitle: '", "true"); @@ -50,7 +50,7 @@ output += expect("radio2.description", "'AXDescription: '"); } else { output += expect("radio3.description", "'AXDescription: radio3'"); - output += expect("radio3.title", "'AXTitle: '"); + output += expect("radio3.title", "'AXTitle: radio3'"); } output += expect("!titleUIElement || titleUIElement.title == 'AXTitle: '", "true"); diff --git a/LayoutTests/accessibility/th-as-title-ui-expected.txt b/LayoutTests/accessibility/th-as-title-ui-expected.txt deleted file mode 100644 index dee2dae7a175..000000000000 --- a/LayoutTests/accessibility/th-as-title-ui-expected.txt +++ /dev/null @@ -1,3 +0,0 @@ -Header for button -Header for button -Pass diff --git a/LayoutTests/accessibility/th-as-title-ui.html b/LayoutTests/accessibility/th-as-title-ui.html deleted file mode 100644 index 37c9e40a80e0..000000000000 --- a/LayoutTests/accessibility/th-as-title-ui.html +++ /dev/null @@ -1,42 +0,0 @@ - - - - - - - - - - - -
Header for button
- - - - - - -
Header for button
- -
- - - - diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations index 2ad470d0186b..48458714ea5b 100644 --- a/LayoutTests/platform/glib/TestExpectations +++ b/LayoutTests/platform/glib/TestExpectations @@ -561,7 +561,6 @@ webkit.org/b/182107 accessibility/aria-combobox-control-owns-elements.html [ Tim webkit.org/b/98363 [ Release ] accessibility/canvas-fallback-content-2.html [ Failure ] webkit.org/b/98372 accessibility/onclick-handlers.html [ Failure ] webkit.org/b/98377 accessibility/textarea-insertion-point-line-number.html [ Timeout ] -webkit.org/b/98380 accessibility/th-as-title-ui.html [ Failure ] webkit.org/b/98382 accessibility/visible-elements.html [ Timeout ] webkit.org/b/215405 accessibility/roles-computedRoleString.html [ Failure ] diff --git a/LayoutTests/platform/mac/accessibility/aria-labelledby-overrides-aria-label-expected.txt b/LayoutTests/platform/mac/accessibility/aria-labelledby-overrides-aria-label-expected.txt index fb6bb6ee92c9..381696d13719 100644 --- a/LayoutTests/platform/mac/accessibility/aria-labelledby-overrides-aria-label-expected.txt +++ b/LayoutTests/platform/mac/accessibility/aria-labelledby-overrides-aria-label-expected.txt @@ -3,10 +3,10 @@ This tests that if aria-labelledby is used, then aria-label attributes are not u Alpha Beta Delta Eta Epsilon Theta usingNone.title: [AXTitle: Alpha] usingNone.description: [AXDescription: ] -usingLabel.title: [AXTitle: ] +usingLabel.title: [AXTitle: Gamma] usingLabel.description: [AXDescription: Gamma] -usingLabelledby.title: [AXTitle: ] +usingLabelledby.title: [AXTitle: Epsilon] usingLabelledby.description: [AXDescription: Epsilon] -usingLabeledby.title: [AXTitle: ] +usingLabeledby.title: [AXTitle: Theta] usingLabeledby.description: [AXDescription: Theta] diff --git a/LayoutTests/platform/mac/accessibility/aria-switch-text-expected.txt b/LayoutTests/platform/mac/accessibility/aria-switch-text-expected.txt index c3a7923e307f..0dcd49126d5c 100644 --- a/LayoutTests/platform/mac/accessibility/aria-switch-text-expected.txt +++ b/LayoutTests/platform/mac/accessibility/aria-switch-text-expected.txt @@ -12,7 +12,7 @@ widget.childrenCount is 0 widget.title is AXTitle: Two widget.description is AXDescription: widget.childrenCount is 0 -widget.title is AXTitle: +widget.title is AXTitle: foo widget.description is AXDescription: foo widget.childrenCount is 0 PASS successfullyParsed is true diff --git a/LayoutTests/platform/mac/accessibility/w3c-svg-name-calculation-expected.txt b/LayoutTests/platform/mac/accessibility/w3c-svg-name-calculation-expected.txt index e6578b8bdf1d..c8195acb3c12 100644 --- a/LayoutTests/platform/mac/accessibility/w3c-svg-name-calculation-expected.txt +++ b/LayoutTests/platform/mac/accessibility/w3c-svg-name-calculation-expected.txt @@ -5,25 +5,25 @@ test1: AXTitle: AXDescription: end AXHelp: abc - AXTitleUIElement: null + AXTitleUIElement: non-null test2: Expected name: end AXTitle: AXDescription: end AXHelp: - AXTitleUIElement: null + AXTitleUIElement: non-null test3: Expected name: end AXTitle: AXDescription: end AXHelp: abc - AXTitleUIElement: null + AXTitleUIElement: non-null test4: Expected name: end AXTitle: AXDescription: end AXHelp: - AXTitleUIElement: null + AXTitleUIElement: non-null test5: Expected name: hello AXTitle: @@ -137,7 +137,7 @@ test23: AXTitle: AXDescription: end AXHelp: - AXTitleUIElement: null + AXTitleUIElement: non-null test24: Expected AXTitle: @@ -179,7 +179,7 @@ test30: AXTitle: AXDescription: boulder AXHelp: - AXTitleUIElement: null + AXTitleUIElement: non-null test31: Expected name: stone AXTitle: diff --git a/Source/WebCore/accessibility/AXCoreObject.cpp b/Source/WebCore/accessibility/AXCoreObject.cpp index a6063dfdf0d3..dadaf1a693f1 100644 --- a/Source/WebCore/accessibility/AXCoreObject.cpp +++ b/Source/WebCore/accessibility/AXCoreObject.cpp @@ -538,4 +538,16 @@ String AXCoreObject::helpTextAttributeValue() const } #endif // PLATFORM(COCOA) +AXCoreObject* AXCoreObject::titleUIElement() const +{ + auto labels = relatedObjects(AXRelationType::LabeledBy); +#if PLATFORM(COCOA) + // We impose the restriction that if there is more than one label, then we should return none. + // FIXME: the behavior should be the same in all platforms. + return labels.size() == 1 ? labels.first().get() : nullptr; +#else + return labels.size() ? labels.first().get() : nullptr; +#endif +} + } // namespace WebCore diff --git a/Source/WebCore/accessibility/AXCoreObject.h b/Source/WebCore/accessibility/AXCoreObject.h index 9432ded6a85f..2ef103e5c0fd 100644 --- a/Source/WebCore/accessibility/AXCoreObject.h +++ b/Source/WebCore/accessibility/AXCoreObject.h @@ -756,7 +756,7 @@ enum class AXRelationType : uint8_t { FlowsTo, Headers, HeaderFor, - LabelledBy, + LabeledBy, LabelFor, OwnedBy, OwnerFor, @@ -1020,7 +1020,7 @@ class AXCoreObject : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr performTextOperation(const AccessibilityTextOperation&) = 0; virtual AccessibilityChildrenVector linkedObjects() const = 0; - virtual AXCoreObject* titleUIElement() const = 0; - virtual AXCoreObject* correspondingLabelForControlElement() const = 0; - virtual AXCoreObject* correspondingControlForLabelElement() const = 0; + virtual AXCoreObject* titleUIElement() const; virtual AXCoreObject* scrollBar(AccessibilityOrientation) = 0; virtual bool inheritsPresentationalRole() const = 0; diff --git a/Source/WebCore/accessibility/AXLogger.cpp b/Source/WebCore/accessibility/AXLogger.cpp index 38e4cb2ede66..01b102c97351 100644 --- a/Source/WebCore/accessibility/AXLogger.cpp +++ b/Source/WebCore/accessibility/AXLogger.cpp @@ -516,8 +516,8 @@ TextStream& operator<<(TextStream& stream, AXRelationType relationType) case AXRelationType::HeaderFor: stream << "HeaderFor"; break; - case AXRelationType::LabelledBy: - stream << "LabelledBy"; + case AXRelationType::LabeledBy: + stream << "LabeledBy"; break; case AXRelationType::LabelFor: stream << "LabelFor"; @@ -620,6 +620,9 @@ TextStream& operator<<(TextStream& stream, AXObjectCache::AXNotification notific case AXObjectCache::AXNotification::AXKeyShortcutsChanged: stream << "AXKeyShortcutsChanged"; break; + case AXObjectCache::AXNotification::AXLabelChanged: + stream << "AXLabelChanged"; + break; case AXObjectCache::AXNotification::AXLanguageChanged: stream << "AXLanguageChanged"; break; @@ -707,9 +710,6 @@ TextStream& operator<<(TextStream& stream, AXObjectCache::AXNotification notific case AXObjectCache::AXNotification::AXScrolledToAnchor: stream << "AXScrolledToAnchor"; break; - case AXObjectCache::AXNotification::AXLabelCreated: - stream << "AXLabelCreated"; - break; case AXObjectCache::AXNotification::AXLiveRegionCreated: stream << "AXLiveRegionCreated"; break; diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp index 024ea342700c..6c7162e4fc95 100644 --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ b/Source/WebCore/accessibility/AXObjectCache.cpp @@ -735,8 +735,9 @@ Ref AXObjectCache::createObjectFromRenderer(RenderObject* r if ((is(renderer) && !renderer->isAnonymous()) || isAccessibilityTableCell(node)) return AccessibilityTableCell::create(renderer); - // progress bar - if (is(renderer) || is(node)) + // Progress indicator. + if (is(renderer) || is(renderer) + || is(node) || is(node)) return AccessibilityProgressIndicator::create(renderer); #if ENABLE(ATTACHMENT_ELEMENT) @@ -744,9 +745,6 @@ Ref AXObjectCache::createObjectFromRenderer(RenderObject* r return AccessibilityAttachment::create(downcast(renderer)); #endif - if (is(renderer) || is(node)) - return AccessibilityProgressIndicator::create(renderer); - // input type=range if (is(renderer)) return AccessibilitySlider::create(renderer); @@ -1054,13 +1052,15 @@ void AXObjectCache::remove(AXID axID) AXTRACE(makeString("AXObjectCache::remove 0x"_s, hex(reinterpret_cast(this)))); AXLOG(makeString("AXID ", axID.loggingString())); - if (!axID) + if (!axID.isValid()) return; auto object = m_objects.take(axID); if (!object) return; + removeRelations(axID); + #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) if (auto tree = AXIsolatedTree::treeForPageID(m_pageID)) tree->removeNode(*object); @@ -1074,9 +1074,11 @@ void AXObjectCache::remove(AXID axID) #endif ASSERT(m_objects.size() >= m_idsInUse.size()); } - + void AXObjectCache::remove(RenderObject* renderer) { + AXTRACE(makeString("AXObjectCache::remove RenderObject* 0x"_s, hex(reinterpret_cast(this)))); + if (!renderer) return; remove(m_renderObjectMapping.take(renderer)); @@ -1084,7 +1086,7 @@ void AXObjectCache::remove(RenderObject* renderer) void AXObjectCache::remove(Node& node) { - AXTRACE(makeString("AXObjectCache::remove 0x"_s, hex(reinterpret_cast(this)))); + AXTRACE(makeString("AXObjectCache::remove Node& 0x"_s, hex(reinterpret_cast(this)))); removeNodeForUse(node); remove(m_nodeObjectMapping.take(&node)); @@ -1161,12 +1163,12 @@ void AXObjectCache::handleTextChanged(AccessibilityObject* object) // If this element supports ARIA live regions, or is part of a region with an ARIA editable role, // then notify the AT of changes. bool notifiedNonNativeTextControl = false; - for (auto* parent = object; parent; parent = parent->parentObject()) { + for (RefPtr parent = object; parent; parent = parent->parentObject()) { if (parent->supportsLiveRegion()) - postLiveRegionChangeNotification(parent); + postLiveRegionChangeNotification(parent.get()); if (!notifiedNonNativeTextControl && parent->isNonNativeTextControl()) { - postNotification(parent, parent->document(), AXValueChanged); + postNotification(parent.get(), parent->document(), AXValueChanged); notifiedNonNativeTextControl = true; } } @@ -1248,6 +1250,9 @@ void AXObjectCache::handleAllDeferredChildrenChanged() void AXObjectCache::handleChildrenChanged(AccessibilityObject& object) { + AXTRACE("AXObjectCache::handleChildrenChanged"_s); + AXLOG(object); + // Handle MenuLists and MenuListPopups as special cases. if (is(object)) { auto& children = object.children(false); @@ -1276,7 +1281,7 @@ void AXObjectCache::handleChildrenChanged(AccessibilityObject& object) // Go up the existing ancestors chain and fire the appropriate notifications. bool shouldUpdateParent = true; - for (auto* parent = &object; parent; parent = parent->parentObjectIfExists()) { + for (RefPtr parent = &object; parent; parent = parent->parentObjectIfExists()) { if (shouldUpdateParent) parent->setNeedsToUpdateChildren(); @@ -1285,19 +1290,19 @@ void AXObjectCache::handleChildrenChanged(AccessibilityObject& object) // Sometimes this function can be called many times within a short period of time, leading to posting too many AXLiveRegionChanged notifications. // To fix this, we use a timer to make sure we only post one notification for the children changes within a pre-defined time interval. if (parent->supportsLiveRegion()) - postLiveRegionChangeNotification(parent); + postLiveRegionChangeNotification(parent.get()); // If this object is an ARIA text control, notify that its value changed. if (parent->isNonNativeTextControl()) { - postNotification(parent, parent->document(), AXValueChanged); + postNotification(parent.get(), parent->document(), AXValueChanged); // 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(axObject.get()), axObject->document(), AXValueChanged); + if (parent->isLabel()) { + // A label's descendant was added or removed. Update its LabelFor relationships. + handleLabelChanged(parent.get()); } } @@ -1346,15 +1351,6 @@ void AXObjectCache::handleLiveRegionCreated(Node* node) postNotification(getOrCreate(node), &document(), AXLiveRegionCreated); } -void AXObjectCache::handleLabelCreated(HTMLLabelElement* element) -{ -#if ENABLE(ACCESSIBILITY_ISOLATED_TREE) - postNotification(getOrCreate(element), nullptr, AXLabelCreated); -#else - UNUSED_PARAM(element); -#endif -} - void AXObjectCache::deferNodeAddedOrRemoved(Node* node) { if (!node) @@ -2445,9 +2441,10 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName& } else if (attrName == disabledAttr) postNotification(element, AXDisabledStateChanged); - else if (attrName == forAttr && is(element)) - handleLabelForChanged(downcast(*element), oldValue); - else if (attrName == requiredAttr) + else if (attrName == forAttr) { + if (RefPtr label = dynamicDowncast(element)) + updateLabelFor(*label); + } else if (attrName == requiredAttr) postNotification(element, AXRequiredStatusChanged); else if (attrName == tabindexAttr) { if (oldValue.isEmpty() || newValue.isEmpty()) { @@ -2619,12 +2616,49 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName& } } -void AXObjectCache::handleLabelForChanged(HTMLLabelElement& label, const AtomString& oldValue) +void AXObjectCache::handleLabelChanged(AccessibilityObject* object) { - RefPtr currentControl = label.control(); - deferTextChangedIfNeeded(currentControl.get()); - RefPtr oldControl = label.treeScope().getElementById(oldValue); - deferTextChangedIfNeeded(oldControl.get()); + AXTRACE("AXObjectCache::handleLabelChanged"_s); + + if (!object) + return; + + if (RefPtr label = dynamicDowncast(object->element())) + updateLabelFor(*label); + else { + auto labeledObjects = object->labelForObjects(); + for (auto& labeledObject : labeledObjects) { + updateLabeledBy(RefPtr { labeledObject->element() }.get()); + postNotification(downcast(labeledObject.get()), &document(), AXValueChanged); + } + } + + postNotification(object, &document(), AXLabelChanged); +} + +void AXObjectCache::updateLabelFor(HTMLLabelElement& label) +{ + removeRelations(label, AXRelationType::LabelFor); + addLabelForRelation(label); +} + +void AXObjectCache::updateLabeledBy(Element* element) +{ + if (!element) + return; + + removeRelations(*element, AXRelationType::LabeledBy); + addRelations(*element, aria_labelledbyAttr); + dirtyIsolatedTreeRelations(); +} + +void AXObjectCache::dirtyIsolatedTreeRelations() +{ +#if ENABLE(ACCESSIBILITY_ISOLATED_TREE) + if (auto tree = AXIsolatedTree::treeForPageID(m_pageID)) + tree->relationsNeedUpdate(true); + startUpdateTreeSnapshotTimer(); +#endif } void AXObjectCache::recomputeIsIgnored(RenderObject* renderer) @@ -3953,7 +3987,11 @@ void AXObjectCache::performDeferredCacheUpdate() if (RefPtr node = weakNode.get()) { handleMenuOpened(node.get()); handleLiveRegionCreated(node.get()); - handleLabelCreated(dynamicDowncast(node.get())); + + if (RefPtr label = dynamicDowncast(node.get())) { + // A label was added or removed. Update its LabelFor relationships. + handleLabelChanged(getOrCreate(label.get())); + } } } m_deferredNodeAddedOrRemovedList.clear(); @@ -4069,7 +4107,7 @@ void AXObjectCache::performDeferredCacheUpdate() handleScrollbarUpdate(scrollView); }); m_deferredScrollbarUpdateChangeList.clear(); - + #if ENABLE(ACCESSIBILITY_ISOLATED_TREE) if (m_deferredRegenerateIsolatedTree) { if (auto tree = AXIsolatedTree::treeForPageID(m_pageID)) { @@ -4134,6 +4172,7 @@ void AXObjectCache::updateIsolatedTree(const Vector updatedObjects; auto updateNode = [&] (RefPtr axObject) { @@ -4190,9 +4229,6 @@ void AXObjectCache::updateIsolatedTree(const VectorqueueNodeUpdate(*notification.first, { AXPropertyName::CanSetFocusAttribute }); break; - case AXLabelCreated: - tree->labelCreated(*notification.first); - break; case AXMaximumValueChanged: tree->queueNodeUpdate(*notification.first, { { AXPropertyName::MaxValueForRange, AXPropertyName::ValueForRange } }); break; @@ -4291,17 +4327,21 @@ void AXObjectCache::updateIsolatedTree(const VectorparentObject(); axParent && axParent->isLabel()) { - if (RefPtr correspondingControl = axParent->correspondingControlForLabelElement()) - updateNode(correspondingControl.get()); + + auto updatedFields = updatedObjects.get(notification.first->objectID()); + if (!updatedFields.dependentProperties) { + updatedObjects.set(notification.first->objectID(), UpdatedFields { updatedFields.children, updatedFields.node, true }); + tree->updateDependentProperties(*notification.first); } break; + } case AXLanguageChanged: case AXRowCountChanged: updateNode(notification.first); @@ -4566,10 +4606,10 @@ AXRelationType AXObjectCache::symmetricRelation(AXRelationType relationType) return AXRelationType::HeaderFor; case AXRelationType::HeaderFor: return AXRelationType::Headers; - case AXRelationType::LabelledBy: + case AXRelationType::LabeledBy: return AXRelationType::LabelFor; case AXRelationType::LabelFor: - return AXRelationType::LabelledBy; + return AXRelationType::LabeledBy; case AXRelationType::OwnedBy: return AXRelationType::OwnerFor; case AXRelationType::OwnerFor: @@ -4595,7 +4635,7 @@ AXRelationType AXObjectCache::attributeToRelationType(const QualifiedName& attri if (attribute == aria_flowtoAttr) return AXRelationType::FlowsTo; if (attribute == aria_labelledbyAttr || attribute == aria_labeledbyAttr) - return AXRelationType::LabelledBy; + return AXRelationType::LabeledBy; if (attribute == aria_ownsAttr) return AXRelationType::OwnerFor; if (attribute == headersAttr) @@ -4607,19 +4647,27 @@ static bool validRelation(void* origin, void* target, AXRelationType relationTyp { if (!origin || !target || relationType == AXRelationType::None) return false; - - if (origin == target && relationType != AXRelationType::LabelledBy) - return false; - - return true; + return origin != target || relationType == AXRelationType::LabeledBy; } void AXObjectCache::addRelation(Element* origin, Element* target, AXRelationType relationType) { + AXTRACE("AXObjectCache::addRelation"_s); + AXLOG(makeString("origin: ", origin->debugDescription(), " target: ", target->debugDescription(), " relationType ", String::number(static_cast(relationType)))); + if (!validRelation(origin, target, relationType)) { ASSERT_NOT_REACHED(); return; } + + if (relationType == AXRelationType::LabelFor) { + // Add a LabelFor relation if the target doesn't have an ARIA label which should take precedence. + if (target->hasAttributeWithoutSynchronization(aria_labelAttr) + || target->hasAttributeWithoutSynchronization(aria_labelledbyAttr) + || target->hasAttributeWithoutSynchronization(aria_labeledbyAttr)) + return; + } + addRelation(getOrCreate(origin, IsPartOfRelation::Yes), getOrCreate(target, IsPartOfRelation::Yes), relationType); } @@ -4713,6 +4761,26 @@ void AXObjectCache::addRelation(AccessibilityObject* origin, AccessibilityObject } } +void AXObjectCache::removeRelations(AXID axID) +{ + auto it = m_relations.find(axID); + if (it == m_relations.end()) + return; + + for (auto relationType : it->value.keys()) { + auto symmetric = symmetricRelation(static_cast(relationType)); + if (symmetric == AXRelationType::None) + continue; + + auto targetIDs = it->value.get(static_cast(relationType)); + for (AXID targetID : targetIDs) + removeRelationByID(targetID, axID, symmetric); + } + + m_relations.remove(it); + dirtyIsolatedTreeRelations(); +} + void AXObjectCache::removeRelations(Element& origin, AXRelationType relationType) { AXTRACE("AXObjectCache::removeRelations"_s); @@ -4739,6 +4807,10 @@ void AXObjectCache::removeRelations(Element& origin, AXRelationType relationType void AXObjectCache::removeRelationByID(AXID originID, AXID targetID, AXRelationType relationType) { + AXTRACE("AXObjectCache::removeRelationByID"_s); + AXLOG(makeString("originID ", originID.loggingString(), " targetID ", targetID.loggingString())); + AXLOG(relationType); + auto relationsIterator = m_relations.find(originID); if (relationsIterator == m_relations.end()) return; @@ -4775,21 +4847,31 @@ void AXObjectCache::updateRelationsForTree(ContainerNode& rootNode) for (const auto& attribute : relationAttributes()) addRelations(element, attribute); + + // In addition to ARIA specified relations, there may be other relevant relations. + // For instance, LabelFor in HTMLLabelElements. + addLabelForRelation(element); } } void AXObjectCache::addRelations(Element& origin, const QualifiedName& attribute) { + if (attribute == aria_labeledbyAttr && origin.hasAttribute(aria_labelledbyAttr)) { + // The attribute name with British spelling should override the one with American spelling. + return; + } + + auto relationType = attributeToRelationType(attribute); if (m_document->settings().ariaReflectionForElementReferencesEnabled()) { if (Element::isElementReflectionAttribute(m_document->settings(), attribute)) { if (auto reflectedElement = origin.getElementAttribute(attribute)) { - addRelation(&origin, reflectedElement.get(), attributeToRelationType(attribute)); + addRelation(&origin, reflectedElement.get(), relationType); return; } } else if (Element::isElementsArrayReflectionAttribute(m_document->settings(), attribute)) { if (auto reflectedElements = origin.getElementsArrayAttribute(attribute)) { for (auto reflectedElement : reflectedElements.value()) - addRelation(&origin, reflectedElement.get(), attributeToRelationType(attribute)); + addRelation(&origin, reflectedElement.get(), relationType); return; } } @@ -4799,7 +4881,7 @@ void AXObjectCache::addRelations(Element& origin, const QualifiedName& attribute if (value.isNull()) { if (auto* defaultARIA = origin.customElementDefaultARIAIfExists()) { for (auto& target : defaultARIA->elementsForAttribute(origin, attribute)) - addRelation(&origin, target.get(), attributeToRelationType(attribute)); + addRelation(&origin, target.get(), relationType); } return; } @@ -4810,7 +4892,24 @@ void AXObjectCache::addRelations(Element& origin, const QualifiedName& attribute if (!target || target == &origin) continue; - addRelation(&origin, target.get(), attributeToRelationType(attribute)); + addRelation(&origin, target.get(), relationType); + } +} + +void AXObjectCache::addLabelForRelation(Element& origin) +{ + // LabelFor relations are established for