In order for these tests to pass: accessibility/button-title-uses-inner-img-alt.html accessibility/focusable-div.html [ Failure ] ...there need to be more GTK accessibility tests that cover textUnderElement in cases where it should return an object replacement character. Then we can try getting rid of a special case for GTK in AccessibilityRenderObject::textUnderElement().
Created attachment 205917 [details] Patch proposal This patch fixes the issue for GTK while (I believe) not disturbing other platforms.
I have just confirmed that fixing this patch gets the test accessibility/listitem-title.html passing.
Created attachment 206160 [details] Patch proposal (In reply to comment #2) > I have just confirmed that fixing this patch gets the test accessibility/listitem-title.html passing. That's right. I've already pushed a commit skipping that one and associating it to this bug, so I'm now updating the patch here to include the unskipping of it, once it gets reviewed.
Comment on attachment 206160 [details] Patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=206160&action=review > Source/WebCore/ChangeLog:3 > + AX: Need more GTK accessibility tests of the object replacement character This title is misleading I think for what it does > Source/WebCore/ChangeLog:8 > + Allow specifying different funtion modes for textUnderElement(), funtion -> function > Source/WebCore/ChangeLog:11 > + element (without omitting any child in the subtree), needed for the portion of this sentence - ", needed for..." Should be its own sentence > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1509 > + if (mode == TextUnderElementModeFullText) I think these modes should be called TextUnderElementIncludeAllChildren TextUnderElementSkipIgnoredChildren (Default) > Source/WebCore/accessibility/AccessibilityRenderObject.h:147 > + virtual String textUnderElement(AccessibilityTextUnderElementMode) const; this should take a default value so that we don't need to modify every instances TextUnderElementMode = ..
Thanks for the review, Chris. I will provide an updated patch asap. (In reply to comment #4) > (From update of attachment 206160 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=206160&action=review > > > Source/WebCore/ChangeLog:3 > > + AX: Need more GTK accessibility tests of the object replacement character > > This title is misleading I think for what it does Yes, that's right. I'm now changing it to something that will hopefully be more accurate > > Source/WebCore/ChangeLog:8 > > + Allow specifying different funtion modes for textUnderElement(), > > funtion -> function Ok. > > Source/WebCore/ChangeLog:11 > > + element (without omitting any child in the subtree), needed for > > the portion of this sentence - ", needed for..." Should be its own sentence Ok. > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1509 > > + if (mode == TextUnderElementModeFullText) > > I think these modes should be called > > TextUnderElementIncludeAllChildren > TextUnderElementSkipIgnoredChildren (Default) Agreed. It definitely sounds better and looks more clear > > Source/WebCore/accessibility/AccessibilityRenderObject.h:147 > > + virtual String textUnderElement(AccessibilityTextUnderElementMode) const; > > this should take a default value so that we don't need to modify every instances > TextUnderElementMode = .. Ok. I'll add that then.
Created attachment 206377 [details] Patch proposal I believe this patch addresses all the issues pointed out before.
Committed r152537: <http://trac.webkit.org/changeset/152537>