Bug 255325

Summary: AX: Properly expose lists that have display:contents list items
Product: WebKit Reporter: Tyler Wilcock <tyler_w>
Component: AccessibilityAssignee: Tyler Wilcock <tyler_w>
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: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Tyler Wilcock 2023-04-12 00:14:06 PDT
<div role="list" id="list">
    <li id="li1" style="display:contents">One</li>
    <li id="li2"style="display:contents">Two</li>
</div>

We don't expose this as a list which is wrong.
Comment 1 Radar WebKit Bug Importer 2023-04-12 00:14:18 PDT
<rdar://problem/107924637>
Comment 2 Tyler Wilcock 2023-04-12 00:21:07 PDT
Created attachment 465862 [details]
Patch
Comment 3 Andres Gonzalez 2023-04-12 07:26:28 PDT
(In reply to Tyler Wilcock from comment #2)
> Created attachment 465862 [details]
> Patch

--- a/Source/WebCore/accessibility/AXObjectCache.cpp
+++ b/Source/WebCore/accessibility/AXObjectCache.cpp

-    RefPtr<AccessibilityObject> newObj = createFromNode(node);
+    RefPtr<AccessibilityObject> newObj = createFromNode(*node);

RefPtr<AccessibilityObject> newObj --> RefPtr newObject

+    if (auto* node = this->node())
+        return node->hasTagName(ulTag);
+    return false;

I prefer the two liner idiom:

    auto* node = this->node();
    return node ? node->hasTagName(ulTag) : false;

+    if (auto* node = this->node())
+        return node->hasTagName(olTag);
+    return false;

Same as above.

+    if (auto* node = this->node())
+        return node->hasTagName(dlTag);
+    return false;

Same as above.

         else if (child->roleValue() == AccessibilityRole::ListItem) {
-            RenderObject* listItem = child->renderer();
-            if (!listItem)
-                continue;
-
             // Rendered list items always count.


Does this comment make sense any more?
Comment 4 Tyler Wilcock 2023-04-12 09:59:59 PDT
Created attachment 465868 [details]
Patch
Comment 5 Tyler Wilcock 2023-04-12 10:01:29 PDT
>          else if (child->roleValue() == AccessibilityRole::ListItem) {
> -            RenderObject* listItem = child->renderer();
> -            if (!listItem)
> -                continue;
> -
>              // Rendered list items always count.
> Does this comment make sense any more?
Just re-read the function and I think it still does since the purpose of the if-statement is to find rendered list items. Fixed all other comments, thanks!
Comment 6 EWS 2023-04-12 15:13:34 PDT
Committed 262889@main (f49113b35bbd): <https://commits.webkit.org/262889@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 465868 [details].