[Gtk] Incorrect rendering of list

[Joanmarie Diggs]

Created attachment 32464 [details]
test case

If you examine the attached test case in Accerciser, you will see that the list is represented as having seven children; it should only have five. The extra children are the paragraphs which should be children of the second list item, but which are each exposed as individual list items/children of the list.

[Joanmarie Diggs]

Xan, I could use a consult. :-)

What is causing the paragraphs to be exposed as ROLE_LIST_ITEM turns out to the fix for bug 25414. Making the change below solves part of the problem, namely it causes the paragraphs to be exposed as role paragraph (good) but they are still children of the list rather than of the list item (bad).

I'll poke around to see what it would take to solve the node/hierarchy issue. BUT, I assume there was a reason you specifically called parentObjectUnignored() instead of parentObject(). What was the reason? Thanks!

--- a/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
+++ b/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
@@ -343,7 +343,7 @@ static AtkRole webkit_accessible_get_role(AtkObject* object)
 
     // WebCore does not seem to have a role for list items
     if (AXObject->isGroup()) {
-        AccessibilityObject* parent = AXObject->parentObjectUnignored();
+        AccessibilityObject* parent = AXObject->parentObject();
         if (parent && parent->isList())
             return ATK_ROLE_LIST_ITEM;
     }

[Joanmarie Diggs]

Created attachment 41812 [details]
add way for platforms to indicate an object should not be ignored

Okay, after some more digging, what's going on appears to be this: Given a list item whose content consists entirely of other HTML elements (e.g. paragraphs), the children of the list item with the content are not ignored; the list item itself, however, is ignored. As a result, we lose the hierarchy we expect, namely:

-> list
   -> list item
   -> list item
      -> paragraph
      -> paragraph
      -> paragraph
   -> list item
   -> list item
   -> list item

The way to solve this problem is to not ignore the list item. :-)

Operating under the assumption that the other platforms somehow don't have this issue, this patch gives platforms the opportunity to indicate that an object should not be ignored before accessibilityIsIgnored() does its thang.

As a related aside, what might be really cool is an accessibility-specific utility along the lines of DumpRenderTree. It would dump out the accessible tree for the platform specified. That way, before going with a platform-specific solution to a problem, one could do a quick sanity check: If it turns out all platforms have the problem, it would be good to solve it across the board.

Anyhoo, review appreciated. Thanks!

[Jan Alonzo]

> The way to solve this problem is to not ignore the list item. :-)

I filed https://bugs.webkit.org/show_bug.cgi?id=25888 a while ago that we don't have a ListItem role in WebCore.
> 
> Operating under the assumption that the other platforms somehow don't have this
> issue, this patch gives platforms the opportunity to indicate that an object
> should not be ignored before accessibilityIsIgnored() does its thang.
> 
> As a related aside, what might be really cool is an accessibility-specific
> utility along the lines of DumpRenderTree. It would dump out the accessible
> tree for the platform specified. That way, before going with a
> platform-specific solution to a problem, one could do a quick sanity check: If
> it turns out all platforms have the problem, it would be good to solve it
> across the board.

There are accessibility tests in LayoutTests/accessibility which we can add tests into (if it's not yet covered).

[Jan Alonzo]

Hi Chris! Can we please ask your thoughts on this? Thanks.

[chris fleizach]

(In reply to comment #4)
> Hi Chris! Can we please ask your thoughts on this? Thanks.

It seems like the !HAVE(AX) should return false instead of true

bool accessibilityPlatformIncludesObject() const { return true; }

although, it probably doesn't matter since it won't get executed

-------

I think this is a good idea, and I'd like the same logic to be applied to the Mac side (then our users won't get wrong counts for the number of list items)

I'm on the fence whether this fix should be applied to all branches or not... however I see the future need to make platform specific decisions, so it should probably stay

[Xan Lopez]

Comment on attachment 41812 [details]
add way for platforms to indicate an object should not be ignored

Ah, this patch is very nice, good idea! Can we get a new version with the !AX case returning false as suggested?

[Joanmarie Diggs]

Created attachment 41854 [details]
Revised so that !AX case returns false

(In reply to comment #6)
> (From update of attachment 41812 [details])
> Ah, this patch is very nice, good idea! Can we get a new version with the !AX
> case returning false as suggested?

Attached. Thanks.

[Xan Lopez]

Comment on attachment 41854 [details]
Revised so that !AX case returns false

r=me

[WebKit Commit Bot]

Comment on attachment 41854 [details]
Revised so that !AX case returns false

Clearing flags on attachment: 41854

Committed r50053: <http://trac.webkit.org/changeset/50053>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.