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

[bugzilla-daemon at webkit.org]

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


Xan Lopez <xan.lopez at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #45740|review?                     |review-
               Flag|                            |




--- Comment #13 from Xan Lopez <xan.lopez at gmail.com>  2010-01-07 03:17:37 PST ---
(From update of attachment 45740)
>From 7de96d0a93b5b44e0dc32bb141a4b52b890e15b1 Mon Sep 17 00:00:00 2001
>From: Joanmarie Diggs <jd at vblockhead.(none)>
>Date: Sat, 2 Jan 2010 11:14:37 -0500
>Subject: [PATCH 2/2] 2010-01-02  Joanmarie Diggs  <joanmarie.diggs at gmail.com>
>
>        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...

>
>        * accessibility/gtk/AccessibilityObjectWrapperAtk.h
>        * accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:
>        (objectAndOffsetUnignored):
>        (webkit_accessible_text_get_caret_offset):
>        * editing/gtk/SelectionControllerGtk.cpp:
>        (SelectionController::notifyAccessibilityForSelectionChange)
>---


> 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'?

Also, can you explain a bit the logic behind the last boolean parameter? 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?


>+
>     // TODO: Verify this for RTL text.
>-    return core(text)->selection().end().offsetInContainerNode();
>+    return offset;
> }
> 
> static AtkAttributeSet* webkit_accessible_text_get_run_attributes(AtkText* text, gint offset, gint* start_offset, gint* end_offset)
>@@ -1729,4 +1737,23 @@ AtkObject* webkit_accessible_get_focused_element(WebKitAccessible* accessible)
>     return focusedObj->wrapper();
> }
> 
>+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?

>+
>+    if (ignoreLinks && realObject->isLink())
>+        realObject = realObject->parentObjectUnignored();
>+
>+    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' :]

>+    offset = range->text().length() + endOffset;
>+}
>+
> #endif // HAVE(ACCESSIBILITY)

Marking r- to clarify my questions/doubts.

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