Skip to content

Commit

Permalink
AX: Refactor the TitleUIElement property using LabeledBy and LabelFor…
Browse files Browse the repository at this point in the history
… relationships.

https://bugs.webkit.org/show_bug.cgi?id=262787
<rdar://problem/116581948>

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
  • Loading branch information
AndresGonzalezApple committed Jan 13, 2024
1 parent c564c28 commit 4ef6169
Show file tree
Hide file tree
Showing 38 changed files with 484 additions and 465 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand Down
11 changes: 6 additions & 5 deletions LayoutTests/accessibility/aria-labelledby-on-password-input.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<script src="../resources/js-test.js"></script>
</head>
<body>

<div>
<button id="button-1" aria-labelledby="password-1">Button One</button>
<button id="button-2">Button Two</button>
Expand All @@ -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";
Expand All @@ -43,12 +44,12 @@
&& 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();
}, 0);
}
</script>
</body>
</html>
</html>
10 changes: 4 additions & 6 deletions LayoutTests/accessibility/aria-labelledby-text-expected.txt
Original file line number Diff line number Diff line change
@@ -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

12 changes: 7 additions & 5 deletions LayoutTests/accessibility/aria-labelledby-text.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
<script src="../resources/js-test.js"></script>
</head>
<body>
<div>

<div id="content">
<button id="button-1" aria-labelledby="label1">Button One</button>
<button id="button-2">Button Two</button>
<p id="label1">Label for Button One</p>
Expand All @@ -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);
}
</script>
</body>
</html>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/accessibility/radio-button-title-label.html
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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");

Expand Down
3 changes: 0 additions & 3 deletions LayoutTests/accessibility/th-as-title-ui-expected.txt

This file was deleted.

42 changes: 0 additions & 42 deletions LayoutTests/accessibility/th-as-title-ui.html

This file was deleted.

1 change: 0 additions & 1 deletion LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -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 ]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -137,7 +137,7 @@ test23:
AXTitle:
AXDescription: end
AXHelp:
AXTitleUIElement: null
AXTitleUIElement: non-null
test24:
Expected
AXTitle:
Expand Down Expand Up @@ -179,7 +179,7 @@ test30:
AXTitle:
AXDescription: boulder
AXHelp:
AXTitleUIElement: null
AXTitleUIElement: non-null
test31:
Expected name: stone
AXTitle:
Expand Down
12 changes: 12 additions & 0 deletions Source/WebCore/accessibility/AXCoreObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 3 additions & 5 deletions Source/WebCore/accessibility/AXCoreObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ enum class AXRelationType : uint8_t {
FlowsTo,
Headers,
HeaderFor,
LabelledBy,
LabeledBy,
LabelFor,
OwnedBy,
OwnerFor,
Expand Down Expand Up @@ -1020,7 +1020,7 @@ class AXCoreObject : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AXCo
AccessibilityChildrenVector errorMessageForObjects() const { return relatedObjects(AXRelationType::ErrorMessageFor); }
AccessibilityChildrenVector flowToObjects() const { return relatedObjects(AXRelationType::FlowsTo); }
AccessibilityChildrenVector flowFromObjects() const { return relatedObjects(AXRelationType::FlowsFrom); }
AccessibilityChildrenVector labelledByObjects() const { return relatedObjects(AXRelationType::LabelledBy); }
AccessibilityChildrenVector labeledByObjects() const { return relatedObjects(AXRelationType::LabeledBy); }
AccessibilityChildrenVector labelForObjects() const { return relatedObjects(AXRelationType::LabelFor); }
AccessibilityChildrenVector ownedObjects() const { return relatedObjects(AXRelationType::OwnerFor); }
AccessibilityChildrenVector owners() const { return relatedObjects(AXRelationType::OwnedBy); }
Expand Down Expand Up @@ -1083,9 +1083,7 @@ class AXCoreObject : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AXCo
virtual Vector<String> 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;
Expand Down
10 changes: 5 additions & 5 deletions Source/WebCore/accessibility/AXLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 4ef6169

Please sign in to comment.