Bug 45383

(In reply to comment #0) > [...] > Prior to the fix for bug #45190, each child of the list was a list item. Now, the list item whose > immediate child is a paragraph is exposed to us as a paragraph. (And the bullet seems to be > missing.) I was able to reproduce this, but also with the patch 45190 reverted so the problem must be somewhere else. Keep looking...

It looks this could be related to stuff done for bug 27085

Created attachment 67918 [details] Patch proposal After some work I think the most simple way to fix this is by just forcing inclusion in accessibility of the list items in the platform-dependant part of the accessibilityIsIgnored() mechanism, as I found the behaviour of not exposing the list item when its immediate child is a non inline object is the expected behaviour, as coded in AccessibilityRenderObject::accessibilityIsIgnored(): [...] if (m_renderer->isBlockFlow() && m_renderer->childrenInline()) return !toRenderBlock(m_renderer)->firstLineBox() && !mouseButtonListener(); [...] As that part of code is what it's making the difference between a <li>plain text</li> item and a <li><p>paragraph text</p></li> item, I thought the best way to fix this for GTK without bothering other ports was the exposed above. Then there's an ugly part in the patch that is required to deal with the special case of the list item markers, which are not exposed to AT's but must be taken into account anyway when considering offets and those kind of things. Hence, now asking for review. Shoot your questions, I'm eager to continue working on this patch.

Btw, while working on this bug I found another strange behaviour with the markers of items with non inline children so I filed this bug: https://bugs.webkit.org/show_bug.cgi?id=45973

Comment on attachment 67918 [details] Patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=67918&action=review > WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp:76 > + // Include all list items, regardless they have or not inline children seems like you already have "role" here, so you shouldn't have to call roleValue() again > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:2031 > + if (role != TableRole && axRenderObject->renderer()->childrenInline()) you should verify renderer() is not nil before accessing inline children > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:2036 > + AccessibilityObject* axRenderChild = axRenderObject->firstChild(); Can't you just access ->children() of the axRenderObject and then check if it has an item in it? that would be the first accessible element.

Created attachment 68206 [details] Patch proposal (In reply to comment #5) > (From update of attachment 67918 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=67918&action=review > > > WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp:76 > > + // Include all list items, regardless they have or not inline children > > seems like you already have "role" here, so you shouldn't have to call roleValue() again Done and also fixed the same mistake in the next line (introduced some commits ago). > > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:2031 > > + if (role != TableRole && axRenderObject->renderer()->childrenInline()) > > you should verify renderer() is not nil before accessing inline children Done. > > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:2036 > > + AccessibilityObject* axRenderChild = axRenderObject->firstChild(); > > Can't you just access ->children() of the axRenderObject and then check if it has an item in it? that would be the first accessible element. Done.

Comment on attachment 68206 [details] Patch proposal can we get a layout test for this behavior. this has the appearance that if something upstream in WebCore changed, it could cause breakage.

Created attachment 68261 [details] Patch proposal (In reply to comment #7) > (From update of attachment 68206 [details]) > can we get a layout test for this behavior. this has the appearance that if something upstream in WebCore changed, it could cause breakage. Attaching a new version of the patch with a Layout Test (GTK specific) to check that list items are always exposed like that

Comment on attachment 68261 [details] Patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=68261&action=review > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:2038 > + if (children.size() > 0) { sorry... this should just be if (children.size())

Created attachment 68289 [details] Patch proposal (In reply to comment #9) > (From update of attachment 68261 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68261&action=review > > > WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:2038 > > + if (children.size() > 0) { > > sorry... > > this should just be > if (children.size()) No problem. Attaching a new patch with that issue fixed.

Comment on attachment 68289 [details] Patch proposal r=me

Comment on attachment 68289 [details] Patch proposal Clearing flags on attachment: 68289 Committed r68023: <http://trac.webkit.org/changeset/68023>

All reviewed patches have been landed. Closing bug.