Bug 130941 - [ATK] No accessible caret-moved events in a href display:block in div
Summary: [ATK] No accessible caret-moved events in a href display:block in div
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, InRadar
Depends on:
Blocks: 25531
  Show dependency treegraph
 
Reported: 2014-03-30 01:27 PDT by Jarek Czekalski
Modified: 2014-06-24 04:09 PDT (History)
10 users (show)

See Also:


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 Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jarek Czekalski 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.
Comment 1 Radar WebKit Bug Importer 2014-03-30 01:27:27 PDT
<rdar://problem/16467594>
Comment 2 Jarek Czekalski 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.
Comment 3 Jarek Czekalski 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.
Comment 4 Jarek Czekalski 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.
Comment 5 Jarek Czekalski 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.
Comment 6 Mario Sanchez Prada 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.
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2014-06-24 04:09:53 PDT
All reviewed patches have been landed.  Closing bug.