- Revision
- 286477
- Author
- tyle...@apple.com
- Date
- 2021-12-02 20:17:30 -0800 (Thu, 02 Dec 2021)
Log Message
AX: In AccessibilityRenderObject::documentLinks, use existing image-map document links instead of always creating new ones
https://bugs.webkit.org/show_bug.cgi?id=233767
Reviewed by Chris Fleizach.
The current implementation of AccessibilityRenderObject::documentLinks always
creates new image-map links rather than using the existing ones created via
AccessibilityRenderObject::addImageMapChildren. This is problematic for two reasons:
1. It is wasteful in terms of memory usage as we will endlessly accumulate
AccessibilityImageMapLink objects in the cache each time
AccessibilityRenderObject::documentLinks is called.
2. It breaks `<area>` document links in isolated tree mode, since the objects created
this way aren't a child of any other object, and thus don't have any representation
in the isolated tree. Concretely, this means AXIsolatedTree::nodeForID never returns
anything for these objects.
In this patch, we try to find the existing image-map link children
in AccessibilityRenderObject::documentLinks before falling back to creating new ones.
Source/WebCore:
Fixes test accessibility/mac/document-links.html in isolated tree mode.
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::documentLinks):
Try to find the existing image-map link children before falling back to creating new ones.
LayoutTests:
* accessibility/mac/document-links-expected.txt:
Update expectations based on new variable names.
* accessibility/mac/document-links.html:
Modernize test and improve variable naming.
* resources/accessibility-helper.js:
(debugEscaped): Added.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (286476 => 286477)
--- trunk/LayoutTests/ChangeLog 2021-12-03 03:51:54 UTC (rev 286476)
+++ trunk/LayoutTests/ChangeLog 2021-12-03 04:17:30 UTC (rev 286477)
@@ -1,3 +1,33 @@
+2021-12-02 Tyler Wilcock <tyle...@apple.com>
+
+ AX: In AccessibilityRenderObject::documentLinks, use existing image-map document links instead of always creating new ones
+ https://bugs.webkit.org/show_bug.cgi?id=233767
+
+ Reviewed by Chris Fleizach.
+
+ The current implementation of AccessibilityRenderObject::documentLinks always
+ creates new image-map links rather than using the existing ones created via
+ AccessibilityRenderObject::addImageMapChildren. This is problematic for two reasons:
+
+ 1. It is wasteful in terms of memory usage as we will endlessly accumulate
+ AccessibilityImageMapLink objects in the cache each time
+ AccessibilityRenderObject::documentLinks is called.
+
+ 2. It breaks `<area>` document links in isolated tree mode, since the objects created
+ this way aren't a child of any other object, and thus don't have any representation
+ in the isolated tree. Concretely, this means AXIsolatedTree::nodeForID never returns
+ anything for these objects.
+
+ In this patch, we try to find the existing image-map link children
+ in AccessibilityRenderObject::documentLinks before falling back to creating new ones.
+
+ * accessibility/mac/document-links-expected.txt:
+ Update expectations based on new variable names.
+ * accessibility/mac/document-links.html:
+ Modernize test and improve variable naming.
+ * resources/accessibility-helper.js:
+ (debugEscaped): Added.
+
2021-12-02 Robert Jenner <jen...@apple.com>
[ Monterey ] http/tests/workers/service/serviceworker-websocket.https.html (layout-test) is constant text failure
Modified: trunk/LayoutTests/accessibility/mac/document-links-expected.txt (286476 => 286477)
--- trunk/LayoutTests/accessibility/mac/document-links-expected.txt 2021-12-03 03:51:54 UTC (rev 286476)
+++ trunk/LayoutTests/accessibility/mac/document-links-expected.txt 2021-12-03 04:17:30 UTC (rev 286477)
@@ -1,5 +1,8 @@
+This test passes if all four links on the page are found.
-----------------------
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
AXHasDocumentRoleAncestor: 0
AXHasWebApplicationAncestor: 0
AXRole: AXLink
@@ -30,7 +33,7 @@
AXFocusableAncestor: <AXLink>
AXEditableAncestor: (null)
AXHighestEditableAncestor: (null)
-AXURL: http://www.apple.com/
+AXURL: LayoutTests/accessibility/mac/document-links.html#Link1
AXAccessKey: (null)
AXLinkRelationshipType:
AXElementBusy: 0
@@ -67,7 +70,7 @@
AXFocusableAncestor: <AXLink>
AXEditableAncestor: (null)
AXHighestEditableAncestor: (null)
-AXURL: http://www.apple.com/
+AXURL: LayoutTests/accessibility/mac/document-links.html#Link2
AXAccessKey: (null)
AXLinkRelationshipType:
AXElementBusy: 0
@@ -82,29 +85,29 @@
AXChildren: <array of size 1>
AXChildrenInNavigationOrder: <array of size 1>
AXHelp:
-AXParent: <AXLink: 'link 3'>
-AXSize: NSSize: {37, 18}
-AXTitle: link 3
+AXParent: <AXLink: 'Link3'>
+AXSize: NSSize: {38, 18}
+AXTitle: Link3
AXDescription:
AXValue:
AXFocused: 0
AXEnabled: 1
-AXWindow: <AXLink: 'link 3'>
+AXWindow: <AXLink: 'Link3'>
AXSelectedTextMarkerRange: (null)
-AXStartTextMarker: <AXLink: 'link 3'>
-AXEndTextMarker: <AXLink: 'link 3'>
+AXStartTextMarker: <AXLink: 'Link3'>
+AXEndTextMarker: <AXLink: 'Link3'>
AXVisited: 0
AXLinkedUIElements: <array of size 0>
AXSelected: 0
AXBlockQuoteLevel: 0
-AXTopLevelUIElement: <AXLink: 'link 3'>
+AXTopLevelUIElement: <AXLink: 'Link3'>
AXLanguage:
AXDOMIdentifier:
AXDOMClassList: <array of size 0>
-AXFocusableAncestor: <AXLink: 'link 3'>
+AXFocusableAncestor: <AXLink: 'Link3'>
AXEditableAncestor: (null)
AXHighestEditableAncestor: (null)
-AXURL: http://webkit.org/
+AXURL: LayoutTests/accessibility/mac/document-links.html#Link3
AXAccessKey: (null)
AXLinkRelationshipType:
AXElementBusy: 0
@@ -118,29 +121,29 @@
AXChildren: <array of size 1>
AXChildrenInNavigationOrder: <array of size 1>
AXHelp:
-AXParent: <AXLink: 'link 4'>
-AXSize: NSSize: {37, 18}
-AXTitle: link 4
+AXParent: <AXLink: 'Link4'>
+AXSize: NSSize: {38, 18}
+AXTitle: Link4
AXDescription:
AXValue:
AXFocused: 0
AXEnabled: 1
-AXWindow: <AXLink: 'link 4'>
+AXWindow: <AXLink: 'Link4'>
AXSelectedTextMarkerRange: (null)
-AXStartTextMarker: <AXLink: 'link 4'>
-AXEndTextMarker: <AXLink: 'link 4'>
+AXStartTextMarker: <AXLink: 'Link4'>
+AXEndTextMarker: <AXLink: 'Link4'>
AXVisited: 0
AXLinkedUIElements: <array of size 0>
AXSelected: 0
AXBlockQuoteLevel: 0
-AXTopLevelUIElement: <AXLink: 'link 4'>
+AXTopLevelUIElement: <AXLink: 'Link4'>
AXLanguage:
AXDOMIdentifier:
AXDOMClassList: <array of size 0>
-AXFocusableAncestor: <AXLink: 'link 4'>
+AXFocusableAncestor: <AXLink: 'Link4'>
AXEditableAncestor: (null)
AXHighestEditableAncestor: (null)
-AXURL: LayoutTests/accessibility/mac/document-links.html#asdf
+AXURL: LayoutTests/accessibility/mac/document-links.html#Link4
AXAccessKey: (null)
AXLinkRelationshipType:
AXElementBusy: 0
@@ -147,5 +150,7 @@
------------
+PASS successfullyParsed is true
- link 3 link 4
+TEST COMPLETE
+ Link3 Link4
Modified: trunk/LayoutTests/accessibility/mac/document-links.html (286476 => 286477)
--- trunk/LayoutTests/accessibility/mac/document-links.html 2021-12-03 03:51:54 UTC (rev 286476)
+++ trunk/LayoutTests/accessibility/mac/document-links.html 2021-12-03 04:17:30 UTC (rev 286477)
@@ -1,33 +1,27 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
<html>
-<script>
- if (window.testRunner)
- testRunner.dumpAsText();
-</script>
-<body id="body">
-
- <div id="result"></div>
-
- <!-- This test passes if it finds all four links in this page -->
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
- <map id="apple" name="imagemap1">
- <area shape="rect" coords="10,10,133,72" href="" title="Link1" />
- <area shape="rect" coords="20,50,133,72" href="" title="Link2" />
- </map>
+<map id="apple" name="imagemap1">
+ <area shape="rect" coords="10,10,133,72" href="" title="Link1" />
+ <area shape="rect" coords="20,50,133,72" href="" title="Link2" />
+</map>
- <img src="" border="0" align="left" usemap="" vspace="1">
+<img src="" border="0" align="left" usemap="" vspace="1">
- <a href="" 3</a>
- <a href="" 4</a>
-
- <script>
- if (window.accessibilityController) {
- var result = document.getElementById("result");
+<a href=""
+<a href=""
- var body = document.getElementById("body");
- body.focus();
- result.innerText += "\n----------------------\n";
- result.innerText += accessibilityController.focusedElement.attributesOfDocumentLinks() + "\n\n";
- }
- </script>
+<script>
+ description("This test passes if all four links on the page are found.")
+
+ let webarea = accessibilityController.rootElement.childAtIndex(0);
+ debugEscaped(webarea.attributesOfDocumentLinks());
+</script>
</body>
</html>
+
Modified: trunk/LayoutTests/resources/accessibility-helper.js (286476 => 286477)
--- trunk/LayoutTests/resources/accessibility-helper.js 2021-12-03 03:51:54 UTC (rev 286476)
+++ trunk/LayoutTests/resources/accessibility-helper.js 2021-12-03 04:17:30 UTC (rev 286477)
@@ -1,3 +1,10 @@
+// This function is necessary when printing AX attributes that are stringified with angle brackets:
+// AXChildren: <array of size 0>
+// `debug` outputs to the `innerHTML` of a generated element, so these brackets must be escaped to be printed.
+function debugEscaped(message) {
+ debug(escapeHTML(message));
+}
+
// Dumps the accessibility tree hierarchy for the given accessibilityObject into
// an element with id="tree", e.g., <pre id="tree"></pre>. In addition, it
// returns a two element array with the first element [0] being false if the
Modified: trunk/Source/WebCore/ChangeLog (286476 => 286477)
--- trunk/Source/WebCore/ChangeLog 2021-12-03 03:51:54 UTC (rev 286476)
+++ trunk/Source/WebCore/ChangeLog 2021-12-03 04:17:30 UTC (rev 286477)
@@ -1,3 +1,32 @@
+2021-12-02 Tyler Wilcock <tyle...@apple.com>
+
+ AX: In AccessibilityRenderObject::documentLinks, use existing image-map document links instead of always creating new ones
+ https://bugs.webkit.org/show_bug.cgi?id=233767
+
+ Reviewed by Chris Fleizach.
+
+ The current implementation of AccessibilityRenderObject::documentLinks always
+ creates new image-map links rather than using the existing ones created via
+ AccessibilityRenderObject::addImageMapChildren. This is problematic for two reasons:
+
+ 1. It is wasteful in terms of memory usage as we will endlessly accumulate
+ AccessibilityImageMapLink objects in the cache each time
+ AccessibilityRenderObject::documentLinks is called.
+
+ 2. It breaks `<area>` document links in isolated tree mode, since the objects created
+ this way aren't a child of any other object, and thus don't have any representation
+ in the isolated tree. Concretely, this means AXIsolatedTree::nodeForID never returns
+ anything for these objects.
+
+ In this patch, we try to find the existing image-map link children
+ in AccessibilityRenderObject::documentLinks before falling back to creating new ones.
+
+ Fixes test accessibility/mac/document-links.html in isolated tree mode.
+
+ * accessibility/AccessibilityRenderObject.cpp:
+ (WebCore::AccessibilityRenderObject::documentLinks):
+ Try to find the existing image-map link children before falling back to creating new ones.
+
2021-12-02 Alan Bujtas <za...@apple.com>
[LFC][IFC] Create display boxes for fragmented inline boxes on bidi boundaries
Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (286476 => 286477)
--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2021-12-03 03:51:54 UTC (rev 286476)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp 2021-12-03 04:17:30 UTC (rev 286477)
@@ -2050,13 +2050,23 @@
} else {
auto* parent = current->parentNode();
if (is<HTMLAreaElement>(*current) && is<HTMLMapElement>(parent)) {
- auto& areaObject = downcast<AccessibilityImageMapLink>(*axObjectCache()->create(AccessibilityRole::ImageMapLink));
- HTMLMapElement& map = downcast<HTMLMapElement>(*parent);
- areaObject.setHTMLAreaElement(downcast<HTMLAreaElement>(current));
- areaObject.setHTMLMapElement(&map);
- areaObject.setParent(accessibilityParentForImageMap(&map));
-
- result.append(&areaObject);
+ auto* parentImage = downcast<HTMLMapElement>(parent)->imageElement();
+ auto* parentImageRenderer = parentImage ? parentImage->renderer() : nullptr;
+ if (auto* parentImageAxObject = document.axObjectCache()->getOrCreate(parentImageRenderer)) {
+ for (const auto& child : parentImageAxObject->children()) {
+ if (is<AccessibilityImageMapLink>(child) && !result.contains(child))
+ result.append(child);
+ }
+ } else {
+ // We couldn't retrieve the already existing image-map links from the parent image, so create a new one.
+ ASSERT_NOT_REACHED("Unexpectedly missing image-map link parent AX object.");
+ auto& areaObject = downcast<AccessibilityImageMapLink>(*axObjectCache()->create(AccessibilityRole::ImageMapLink));
+ auto& map = downcast<HTMLMapElement>(*parent);
+ areaObject.setHTMLAreaElement(downcast<HTMLAreaElement>(current));
+ areaObject.setHTMLMapElement(&map);
+ areaObject.setParent(accessibilityParentForImageMap(&map));
+ result.append(&areaObject);
+ }
}
}
}