[Webkit-unassigned] [Bug 114871] [GTK] Reimplement atk_text_get_text_*_offset for WORD boundaries

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Martin Robinson <mrobinson at webkit.org>  2013-06-20 13:18:01 PST ---
(From update of attachment 203441)
View in context: https://bugs.webkit.org/attachment.cgi?id=203441&action=review

Sorry for the late review. I've a couple questions about the patch. Nice testing!

> Source/WebCore/ChangeLog:50
> +

Double changelog here.

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:473
> +static gint offsetAdjustmentForObject(const AccessibilityObject* object)

Maybe call this listItemMarkerOffsetForObject?

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:489
> +static gint webCoreOffsetToAtkOffset(const AccessibilityObject* object, gint offset)

You shouldn't use glib types for private functions. So gint -> int and for the rest of this patch.

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:544
> +    Node* referenceNode = rangeStartNode->isInShadowTree() ? rangeStartNode->highestAncestor() : node;

Does the shadow DOM still exist?

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:644
> +    // word following.

I'm curious why you don't use things like startOfWord and endOfWord to do these tasks.

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:691
> +        if (isStartOfWord(originalPosition) && isWhitespace(originalPosition.characterBefore()))

In what situations can something be a start of a word and not prefixed by whitespace. Does this account for if this position is the very first position in the document?

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:700
> +        endPosition = nextWordPosition(startPosition);

Why not endOfWord?

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:806
> +        return webkitAccessibleTextGetWord(text, offset, boundaryType, textPosition, startOffset, endOffset);

webkitAccessibleTextGetWord -> webkitAccessibleTextGetAtWordBoundary?

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:813
> +    // has been properly implemented without using Pango/Cairo.
>      GailOffsetType offsetType;
>      switch (textPosition) {
>      case GetTextPositionBefore:

I guess this isn't the last one?

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