[Webkit-unassigned] [Bug 30883] [Gtk] Implement AtkText for HTML elements which contain text

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Jan 9 12:57:18 PST 2010


https://bugs.webkit.org/show_bug.cgi?id=30883





--- Comment #14 from Joanmarie Diggs <joanmarie.diggs at gmail.com>  2010-01-09 12:57:17 PST ---
Thanks for the review! New patch to follow shortly.

>        Reviewed by NOBODY (OOPS!).
> >
> >        https://bugs.webkit.org/show_bug.cgi?id=30883
> >        [Gtk] Implement AtkText for HTML elements which contain text
> 
> We need some kind of explanation here of what the patch does...

Will do.

> static gint webkit_accessible_text_get_caret_offset(AtkText* text)
> > {
> >+    AccessibilityObject* coreObject = core(text);
> >+    RenderObject* focusedNode = coreObject->selection().end().node()->renderer();
> >+    AccessibilityObject* focusedObject = coreObject->document()->axObjectCache()->getOrCreate(focusedNode);
> >+
> >+    AccessibilityObject* object;
> >+    int offset;
> >+    objectAndOffsetUnignored(focusedObject, object, offset, !coreObject->isLink());
> 
> Not sure I see a reason to not use the return value of the function for, say,
> the 'realObject'?

Will change.

> Also, can you explain a bit the logic behind the last boolean parameter?

Will add comments to the code which will hopefully clarify/remind us of what's
taking place.

> In the
> function below I see that it's used to decide whether to go one level up or not
> when the object containing the text is a link, but I'm not sure I get why here
> you pass !isLink(). If it's not a link you want to ignore them?

Exactly. :-)

Here's the deal: In flattening the hierarchy w.r.t. text objects, we're no
longer exposing all of those RenderText objects to ATs in the form of
ATK_ROLE_TEXT. We are still exposing links however. (Ultimately, that too may
be something that we can "collapse" fully into the parent object, but I'm not
yet sure if that's a good idea.)

Links now implement the AtkText just like paragraphs, etc. So given:

<body>
<p>This is a <a href="foo">test</a>.</p>
</body>

We have this hierarchy:

-> Document Frame
   -> Paragraph (accessible text: "This is a test.")
      -> Link (accessible text: "test")

Thus an AT interested in the offset for an AtkText object might be asking us
for the offset in the paragraph, or it may be asking for the offset in the
link. As far as WebKit is concerned, however, the caret is in neither the
paragraph nor the link; it's in a RenderText object which is now ignored due to
the flattening of the hierarchy.

As a general rule, we can (and do) call parentObjectUnignored to find out the
real object as far as the AT is concerned. Problem (normally) solved. BUT: At
the moment, links are not ignored objects; they're exposed objects. So the
question is, do we report the caret offset w.r.t. the link or w.r.t. the full
paragraph? The answer depends on what AtkText object the caller specified. If
that object is a link, then we don't want to ignore links. If that object is
not a link, but instead the parent of a link, we do want to ignore links and
work our way up to the unignored parent of the link, in this case the
paragraph.

Regarding the caret-moved events to report, I had to make a judgment call. Do
we:

1. Emit events for both objects?
2. Emit events for only the paragraph?
3. Emit events for only the link?

The more events which get emitted, the more events an AT has to process. Plus,
when caret navigation moves into a link, that link gets a focus rectangle and
should emit a focus: and object:state-changed:focused event and also claim
ATK_STATE_FOCUSED. In other words, ATs already will know that the user just
arrowed into a link. And if they want the caret offset w.r.t. that link, they
can always ask for it as described above. Therefore, I decided that the thing
to do in terms of emitting caret moved events (and also selection changed
events), is to just emit events for the paragraph and not for the link.

>+void objectAndOffsetUnignored(AccessibilityObject* coreObject, AccessibilityObject*& realObject, int& offset, bool ignoreLinks)
> >+{
> >+
> >+    Node* endNode = static_cast<AccessibilityRenderObject*>(coreObject)->renderer()->node();
> >+    int endOffset = coreObject->selection().end().offsetInContainerNode();
> >+
> >+    realObject = coreObject;
> >+    if (realObject->accessibilityIsIgnored())
> >+        realObject = realObject->parentObjectUnignored();
> 
> This can only happen once? Any specific cases in mind?

Yes. All cases. :-) parentObjectUnignored() by definition returns the first
parent object for which accessibilityIsIgnored() is false. Therefore, if
realObject->accessibilityIsIgnored() is true, setting realObject to
realObject->parentObjectUnignored() will give us the object we should report to
the AT. Unless, of course, realObject is now a link (because we don't ignore
links normally) and the caller wants to ignore links for one of the reasons
described above. Therefore, we do this bit:

> >+    if (ignoreLinks && realObject->isLink())
> >+        realObject = realObject->parentObjectUnignored();

And we should be good, I believe.

> >+    RefPtr<Range> range = rangeOfContents(static_cast<AccessibilityRenderObject*>(realObject)->renderer()->node());
> >+    ExceptionCode ec = 0;
> >+    range->setEndBefore(endNode, ec);
> 
> We all know this will fail one day and we will say 'oh damn, should have
> checked that value' :]

Hehehe. Yeah, I know.... But I wasn't sure how it would fail or what I should
do in response. And I grepped through the code and didn't see anything
especially enlightening done in similar calls. So... If you could elaborate on
what you think I should do here, I'd be happy to do it.

Thanks!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list