Title: [286477] trunk
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);
+                }
             }
         }
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to