Bug 130941

Summary: [ATK] No accessible caret-moved events in a href display:block in div
Product: WebKit Reporter: Jarek Czekalski <jarekczek>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, mario, samuel_white, webkit-bug-importer
Priority: P2 Keywords: Gtk, InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 25531    
Attachments:
Description Flags
html file to reproduce the bug
none
a test case
none
fix caret in a block, v1.00 none

Jarek Czekalski
Reported 2014-03-30 01:27:13 PDT
Created attachment 228124 [details] html file to reproduce the bug Gnome-help contains "a" tag with css style display:block as the only element inside a div. In such cases webkit does not emit "caret-moved" events. While chasing the bug I discovered also other cases were the event is not sent. I attach an example html file with 8 lines. It shows all the possibilities of "a" tags considering 3 states: 1. with href attribute or not 2. with display:block style or not 3. as a single element inside a div or without a div The 4 lines that exhibit the bug are marked with "-" at the beginning. To reproduce the bug use means similar as those in bug #76069, bug #72811. I launch yelp with block.html and run the python script like the one from https://bugs.webkit.org/attachment.cgi?id=115151 I will slowly work on more internal info about the bug and on providing the test cases (testatk.c). For now I discovered from Source/WebCore/editing/atk/FrameSelectionAtk.cpp that: Errors in lines 2 and 6 are rejected by objectFocusedAndCaretOffsetUnignored. Errors in lines 5 and 7 are rejected by the condition in emitTextSelectionChange.
Attachments
html file to reproduce the bug (516 bytes, text/html)
2014-03-30 01:27 PDT, Jarek Czekalski 		no flags
Details
a test case (134 bytes, text/html)
2014-04-21 04:44 PDT, Jarek Czekalski 		no flags
Details
fix caret in a block, v1.00 (5.25 KB, patch)
2014-06-17 10:07 PDT, Jarek Czekalski 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2014-03-30 01:27:27 PDT
<rdar://problem/16467594>
Jarek Czekalski
Comment 2 2014-04-21 04:44:08 PDT
Created attachment 229800 [details] a test case Actually the html file I attached earlier demonstrates a different bug, that I reported as bug #131933. So I submit a new test case for the bug here, which actually resembles the problem in gnome help. This is the clue of the bug: <div> <p>Para 1</p> <a style="display:block;" href="anything">An a tag with href</a> <p>Para 2</p> </div> To the bug #131933 I attached a testatk.c patch which detects both problems. This patch is hard to divide into 2 pieces. Actually the testatk.c patch contains additional divs in the test case shown above, they should rather be removed.
Jarek Czekalski
Comment 3 2014-04-26 11:25:25 PDT
The crucial part is this: // Don't ignore links if the offset is being requested for a link. if (!referenceObject->isLink() && firstUnignoredParent->isLink()) firstUnignoredParent = firstUnignoredParent->parentObjectUnignored(); objectFocusedAndCaretOffsetUnignored in Source/WebCore/accessibility/atk/WebKitAccessibleWrapperAtk.cpp, line 1333 Our accessibility tree branch is as follows: web_document -> link -> text text is the role for which objectFocusedAndCaretOffsetUnignored is executed. It is not the link, it's the link's child. So the condition above results in dropping the caret moved event. Why does this happen? Traces lead to r53072, which is commented as follows: Adjust the caret offset and object with focus to reflect the unignored parent of the static text object which contains the caret. This is necessary because the static text objects are no longer being exposed to ATs. That's seems to contradict the current actual situation, where something under the link, being text, is exposed to atk. How do we deal with this? Why do we need to ignore links at all? Maybe some answers are in bug #30883 (related with r53072), but I have not read it yet. Just reporting the results so far.
Jarek Czekalski
Comment 4 2014-04-27 01:39:26 PDT
I was wrong. The atk text object, whose parent is the link, is not exposed. That is the link has 0 children. I tried to check if renderer()->isRenderObject() to stop ignoring block links. This works for the provided test case. Unfortunately gnome documentation still does not work, so I have to prepare another test case and think of a patch then.
Jarek Czekalski
Comment 5 2014-06-17 10:07:33 PDT
Created attachment 233237 [details] fix caret in a block, v1.00 Hello again, Mario! I'm attaching another short patch. Please consider whether the suggested solution is correct. It would be cleaner to use renderer()->isBlock instead of !renderer()->isInline(), but only the latter syntax may be easily merged into stable branch. After this patch yelp title page becomes accessible in webkit1. I'm planning to ask for stable branch merge for this patch, which should be applied after: 1. Bug #132527 2. the hot fix for the above patch (contained in the ticket) 3. Bug #132349 4. Bug #130941 (the one here) These patches apply almost cleanly to the stable branch, a small adjustment is needed for the first one. Thanks in advance for the review.
Mario Sanchez Prada
Comment 6 2014-06-24 04:06:54 PDT
Comment on attachment 233237 [details] fix caret in a block, v1.00 Thanks for the patch, it does look good to me.
WebKit Commit Bot
Comment 7 2014-06-24 04:09:47 PDT
Comment on attachment 233237 [details] fix caret in a block, v1.00 Clearing flags on attachment: 233237 Committed r170359: <http://trac.webkit.org/changeset/170359>
WebKit Commit Bot
Comment 8 2014-06-24 04:09:53 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.