[Webkit-unassigned] [Bug 120638] [ATK] Implement new API in AtkText: atk_text_get_string_at_offset()

[bugzilla-daemon at webkit.org]

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


Gustavo Noronha (kov) <gns at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #210383|review?                     |review+
               Flag|                            |




--- Comment #3 from Gustavo Noronha (kov) <gns at gnome.org>  2013-10-09 04:37:28 PST ---
(From update of attachment 210383)
View in context: https://bugs.webkit.org/attachment.cgi?id=210383&action=review

Looks sane to me =)

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:1106
> +static gchar* webkitAccessibleTextGetStringAtOffset(AtkText* text, gint offset, AtkTextGranularity granularity, gint* startOffset, gint* endOffset)

Nit: we have been using gints elsewhere in the a11y implementation, but I'd rather use the regular types where possible to avoid confusion.

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:1110
> +    // ATK_TEXT_BOUNDARY_*_END boundaries, so for now we just need to translate the

s/END/START/

> Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp:1140
> +        g_assert_not_reached();

ASSERT_NOT_REACHED() instead?

> Source/WebKit/gtk/tests/testatk.c:1402
> +    webkit_web_view_set_settings(webView, settings);

This should not be needed since you're using a settings object you got from the view instead of creating a new one. Out of curiosity, why do you have to enable caret browsing here? I thought you were not using the caret to decide where to get text from.

> Source/WebKit/gtk/tests/testatk.c:1450
> +    webkit_web_view_set_settings(webView, settings);

Ditto.

> Tools/gtk/jhbuild.modules:234
> -    <branch module="pub/GNOME/sources/atk/2.8/atk-2.8.0.tar.xz" version="2.8.0"
> +    <branch module="pub/GNOME/sources/atk/2.9/atk-2.9.4.tar.xz" version="2.9.4"
>              repo="ftp.gnome.org"
> -            hash="sha256:b22519176226f3e07cf6d932b77852e6b6be4780977770704b32d0f4e0686df4"/>
> +            hash="sha256:755c9582ca7cf01e5eaa0ac972376877eb365902f388262c21ea3425e0c4d631"/>

This will help with testing I assume? Do we not need new versions of the at-spi stuff as well?

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