2010-09-13 Mario Sanchez Prada <msanchez@igalia.com>
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Sep 2010 12:29:26 +0000 (12:29 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 13 Sep 2010 12:29:26 +0000 (12:29 +0000)
        Reviewed by Martin Robinson.

        [GTK] ATs should be able to select/unselect text
        https://bugs.webkit.org/show_bug.cgi?id=25673

        Implement AtkText's setSelection and removeSelection functions

        * accessibility/AccessibilityObject.cpp:
        (WebCore::AccessibilityObject::visiblePositionRangeForRange):
        Moved some GTK specific code from a ifdef-endif region to
        AccessibilityObjectAtk.cpp
        * accessibility/AccessibilityObject.h:
        * accessibility/gtk/AccessibilityObjectAtk.cpp:
        (WebCore::AccessibilityObject::getLengthForTextRange): New.
        * accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:
        (webkit_accessible_text_remove_selection): Implemented following
        the lead of GAIL's implementation of the AtkText interface.
        (webkit_accessible_text_set_selection): Implemented following
        the lead of GAIL's implementation of the AtkText interface.
        (webkit_accessible_text_set_caret_offset): Changed to directly use
        visiblePositionRangeForRange now that there's no longer a problem
        with that, as it was in the past (only worked for text controls).
2010-09-13  Mario Sanchez Prada  <msanchez@igalia.com>

        Reviewed by Martin Robinson.

        [GTK] Provide unit tests for AtkText's text selection functions
        https://bugs.webkit.org/show_bug.cgi?id=43919

        New tests to check getting, setting and removing text selections

        * tests/testatk.c:
        (testWekitAtkTextSelections): New unit tests to check all the text
        selection related functions altogether through a single test
        function.
        (main):

        Make sure that code dependant on getting information from the
        clipboard gets executed only when there's a GDK window associated
        to the webview widget, as that's not the case when executing the
        unit tests (the wedbview is not inside of any toplevel window) and
        will make the tests crash if not taken into account.

        * WebCoreSupport/EditorClientGtk.cpp:
        (WebKit::EditorClient::respondToChangedSelection):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@67383 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/accessibility/AccessibilityObject.cpp
WebCore/accessibility/AccessibilityObject.h
WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp
WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp
WebKit/gtk/ChangeLog
WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp
WebKit/gtk/tests/testatk.c

index 64be38e..c67bc2e 100644 (file)
@@ -1,3 +1,28 @@
+2010-09-13  Mario Sanchez Prada  <msanchez@igalia.com>
+
+        Reviewed by Martin Robinson.
+
+        [GTK] ATs should be able to select/unselect text
+        https://bugs.webkit.org/show_bug.cgi?id=25673
+
+        Implement AtkText's setSelection and removeSelection functions
+
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::AccessibilityObject::visiblePositionRangeForRange):
+        Moved some GTK specific code from a ifdef-endif region to
+        AccessibilityObjectAtk.cpp
+        * accessibility/AccessibilityObject.h:
+        * accessibility/gtk/AccessibilityObjectAtk.cpp:
+        (WebCore::AccessibilityObject::getLengthForTextRange): New.
+        * accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:
+        (webkit_accessible_text_remove_selection): Implemented following
+        the lead of GAIL's implementation of the AtkText interface.
+        (webkit_accessible_text_set_selection): Implemented following
+        the lead of GAIL's implementation of the AtkText interface.
+        (webkit_accessible_text_set_caret_offset): Changed to directly use
+        visiblePositionRangeForRange now that there's no longer a problem
+        with that, as it was in the past (only worked for text controls).
+
 2010-08-27  Kenneth Rohde Christiansen  <kenneth@webkit.org>
 
         Reviewed by Antti Koivisto.
index a6bb033..3ddcdc5 100644 (file)
@@ -373,22 +373,7 @@ VisiblePositionRange AccessibilityObject::styleRangeForPosition(const VisiblePos
 // NOTE: Consider providing this utility method as AX API
 VisiblePositionRange AccessibilityObject::visiblePositionRangeForRange(const PlainTextRange& range) const
 {
-    unsigned textLength = text().length();
-#if PLATFORM(GTK)
-    // Gtk ATs need this for all text objects; not just text controls.
-    if (!textLength) {
-        Node* node = this->node();
-        RenderObject* renderer = node ? node->renderer() : 0;
-        if (renderer && renderer->isText()) {
-            RenderText* renderText = toRenderText(renderer);
-            textLength = renderText ? renderText->textLength() : 0;
-        }
-        // Get the text length from the elements under the
-        // accessibility object if the value is still zero.
-        if (!textLength && allowsTextRanges())
-            textLength = textUnderElement().length();
-    }
-#endif
+    unsigned textLength = getLengthForTextRange();
     if (range.start + range.length > textLength)
         return VisiblePositionRange();
 
index 0e44d1b..e8f6d1b 100644 (file)
@@ -574,8 +574,10 @@ protected:
     
 #if PLATFORM(GTK)
     bool allowsTextRanges() const;
+    unsigned getLengthForTextRange() const;
 #else
     bool allowsTextRanges() const { return isTextControl(); }
+    unsigned getLengthForTextRange() const { return text().length(); }
 #endif
 
 #if PLATFORM(MAC)
index b22a51e..7f9fd1a 100644 (file)
@@ -20,6 +20,8 @@
 
 #include "config.h"
 #include "AccessibilityObject.h"
+#include "RenderObject.h"
+#include "RenderText.h"
 
 #include <glib-object.h>
 
@@ -102,6 +104,29 @@ bool AccessibilityObject::allowsTextRanges() const
     return isTextControl() || isWebArea() || isGroup() || isLink() || isHeading();
 }
 
+unsigned AccessibilityObject::getLengthForTextRange() const
+{
+    unsigned textLength = text().length();
+
+    if (textLength)
+        return textLength;
+
+    // Gtk ATs need this for all text objects; not just text controls.
+    Node* node = this->node();
+    RenderObject* renderer = node ? node->renderer() : 0;
+    if (renderer && renderer->isText()) {
+        RenderText* renderText = toRenderText(renderer);
+        textLength = renderText ? renderText->textLength() : 0;
+    }
+
+    // Get the text length from the elements under the
+    // accessibility object if the value is still zero.
+    if (!textLength && allowsTextRanges())
+        textLength = textUnderElement().length();
+
+    return textLength;
+}
+
 } // namespace WebCore
 
 #endif // HAVE(ACCESSIBILITY)
index e818113..980e16e 100644 (file)
@@ -1459,32 +1459,51 @@ static gboolean webkit_accessible_text_add_selection(AtkText* text, gint start_o
     return FALSE;
 }
 
-static gboolean webkit_accessible_text_remove_selection(AtkText* text, gint selection_num)
+static gboolean webkit_accessible_text_set_selection(AtkText* text, gint selectionNum, gint startOffset, gint endOffset)
 {
-    notImplemented();
-    return FALSE;
+    // WebCore does not support multiple selection, so anything but 0 does not make sense for now.
+    if (selectionNum)
+        return FALSE;
+
+    // Consider -1 and out-of-bound values and correct them to length
+    gint textCount = webkit_accessible_text_get_character_count(text);
+    if (startOffset < 0 || startOffset > textCount)
+        startOffset = textCount;
+    if (endOffset < 0 || endOffset > textCount)
+        endOffset = textCount;
+
+    AccessibilityObject* coreObject = core(text);
+    PlainTextRange textRange(startOffset, endOffset - startOffset);
+    VisiblePositionRange range = coreObject->visiblePositionRangeForRange(textRange);
+    coreObject->setSelectedVisiblePositionRange(range);
+
+    return TRUE;
 }
 
-static gboolean webkit_accessible_text_set_selection(AtkText* text, gint selection_num, gint start_offset, gint end_offset)
+static gboolean webkit_accessible_text_remove_selection(AtkText* text, gint selectionNum)
 {
-    notImplemented();
-    return FALSE;
+    // WebCore does not support multiple selection, so anything but 0 does not make sense for now.
+    if (selectionNum)
+        return FALSE;
+
+    // Do nothing if current selection doesn't belong to the object
+    if (!webkit_accessible_text_get_n_selections(text))
+        return FALSE;
+
+    // Set a new 0-sized selection to the caret position, in order
+    // to simulate selection removal (GAIL style)
+    gint caretOffset = webkit_accessible_text_get_caret_offset(text);
+    return webkit_accessible_text_set_selection(text, selectionNum, caretOffset, caretOffset);
 }
 
 static gboolean webkit_accessible_text_set_caret_offset(AtkText* text, gint offset)
 {
     AccessibilityObject* coreObject = core(text);
 
-    // FIXME: We need to reimplement visiblePositionRangeForRange here
-    // because the actual function checks the offset is within the
-    // boundaries of text().length(), but text() only works for text
-    // controls...
-    VisiblePosition startPosition = coreObject->visiblePositionForIndex(offset);
-    startPosition.setAffinity(DOWNSTREAM);
-    VisiblePosition endPosition = coreObject->visiblePositionForIndex(offset);
-    VisiblePositionRange range = VisiblePositionRange(startPosition, endPosition);
-
+    PlainTextRange textRange(offset, 0);
+    VisiblePositionRange range = coreObject->visiblePositionRangeForRange(textRange);
     coreObject->setSelectedVisiblePositionRange(range);
+
     return TRUE;
 }
 
index 5afb911..a77b2d4 100644 (file)
@@ -1,3 +1,27 @@
+2010-09-13  Mario Sanchez Prada  <msanchez@igalia.com>
+
+        Reviewed by Martin Robinson.
+
+        [GTK] Provide unit tests for AtkText's text selection functions
+        https://bugs.webkit.org/show_bug.cgi?id=43919
+
+        New tests to check getting, setting and removing text selections
+
+        * tests/testatk.c:
+        (testWekitAtkTextSelections): New unit tests to check all the text
+        selection related functions altogether through a single test
+        function.
+        (main):
+
+        Make sure that code dependant on getting information from the
+        clipboard gets executed only when there's a GDK window associated
+        to the webview widget, as that's not the case when executing the
+        unit tests (the wedbview is not inside of any toplevel window) and
+        will make the tests crash if not taken into account.
+
+        * WebCoreSupport/EditorClientGtk.cpp:
+        (WebKit::EditorClient::respondToChangedSelection):
+
 2010-09-11  Xan Lopez  <xlopez@igalia.com>
 
         Reviewed by Martin Robinson.
index 8a019b4..b1973bc 100644 (file)
@@ -341,6 +341,31 @@ static void collapseSelection(GtkClipboard* clipboard, WebKitWebView* webView)
     frame->selection()->setBase(frame->selection()->extent(), frame->selection()->affinity());
 }
 
+#if PLATFORM(X11)
+static void setSelectionPrimaryClipboardIfNeeded(WebKitWebView* webView)
+{
+    if (!gtk_widget_has_screen(GTK_WIDGET(webView)))
+        return;
+
+    GtkClipboard* clipboard = gtk_widget_get_clipboard(GTK_WIDGET(webView), GDK_SELECTION_PRIMARY);
+    DataObjectGtk* dataObject = DataObjectGtk::forClipboard(clipboard);
+    WebCore::Page* corePage = core(webView);
+    Frame* targetFrame = corePage->focusController()->focusedOrMainFrame();
+
+    if (!targetFrame->selection()->isRange())
+        return;
+
+    dataObject->clear();
+    dataObject->setRange(targetFrame->selection()->toNormalizedRange());
+
+    viewSettingClipboard = webView;
+    GClosure* callback = g_cclosure_new_object(G_CALLBACK(collapseSelection), G_OBJECT(webView));
+    g_closure_set_marshal(callback, g_cclosure_marshal_VOID__VOID);
+    pasteboardHelperInstance()->writeClipboardContents(clipboard, callback);
+    viewSettingClipboard = 0;
+}
+#endif
+
 void EditorClient::respondToChangedSelection()
 {
     WebKitWebViewPrivate* priv = m_webView->priv;
@@ -354,19 +379,7 @@ void EditorClient::respondToChangedSelection()
         return;
 
 #if PLATFORM(X11)
-    GtkClipboard* clipboard = gtk_widget_get_clipboard(GTK_WIDGET(m_webView), GDK_SELECTION_PRIMARY);
-    DataObjectGtk* dataObject = DataObjectGtk::forClipboard(clipboard);
-
-    if (targetFrame->selection()->isRange()) {
-        dataObject->clear();
-        dataObject->setRange(targetFrame->selection()->toNormalizedRange());
-
-        viewSettingClipboard = m_webView;
-        GClosure* callback = g_cclosure_new_object(G_CALLBACK(collapseSelection), G_OBJECT(m_webView));
-        g_closure_set_marshal(callback, g_cclosure_marshal_VOID__VOID);
-        pasteboardHelperInstance()->writeClipboardContents(clipboard, callback);
-        viewSettingClipboard = 0;
-    }
+    setSelectionPrimaryClipboardIfNeeded(m_webView);
 #endif
 
     if (!targetFrame->editor()->hasComposition())
index dbe88b0..9ca7c05 100644 (file)
@@ -44,10 +44,12 @@ static const char* contentsInTable = "<html><body><table><tr><td>foo</td><td>bar
 
 static const char* contentsInTableWithHeaders = "<html><body><table><tr><th>foo</th><th>bar</th><th colspan='2'>baz</th></tr><tr><th>qux</th><td>1</td><td>2</td><td>3</td></tr><tr><th rowspan='2'>quux</th><td>4</td><td>5</td><td>6</td></tr><tr><td>6</td><td>7</td><td>8</td></tr><tr><th>corge</th><td>9</td><td>10</td><td>11</td></tr></table><table><tr><td>1</td><td>2</td></tr><tr><td>3</td><td>4</td></tr></table></body></html>";
 
-static const char* textWithAttributes = "<html><head><style>.st1 {font-family: monospace; color:rgb(120,121,122);} .st2 {text-decoration:underline; background-color:rgb(80,81,82);}</style></head><body><p style=\"font-size:14; text-align:right;\">This is the <i>first</i><b> sentence of this text.</b></p><p class=\"st1\">This sentence should have an style applied <span class=\"st2\">and this part should have another one</span>.</p><p>x<sub>1</sub><sup>2</sup>=x<sub>2</sub><sup>3</sup></p><p style=\"text-align:center;\">This sentence is the <strike>last</strike> one.</p></body></html>";
-
 static const char* listsOfItems = "<html><body><ul><li>text only</li><li><a href='foo'>link only</a></li><li>text and a <a href='bar'>link</a></li></ul><ol><li>text only</li><li><a href='foo'>link only</a></li><li>text and a <a href='bar'>link</a></li></ol></body></html>";
 
+static const char* textForSelections = "<html><body><p>A paragraph with plain text</p><p>A paragraph with <a href='http://webkit.org'>a link</a> in the middle</p></body></html>";
+
+static const char* textWithAttributes = "<html><head><style>.st1 {font-family: monospace; color:rgb(120,121,122);} .st2 {text-decoration:underline; background-color:rgb(80,81,82);}</style></head><body><p style=\"font-size:14; text-align:right;\">This is the <i>first</i><b> sentence of this text.</b></p><p class=\"st1\">This sentence should have an style applied <span class=\"st2\">and this part should have another one</span>.</p><p>x<sub>1</sub><sup>2</sup>=x<sub>2</sub><sup>3</sup></p><p style=\"text-align:center;\">This sentence is the <strike>last</strike> one.</p></body></html>";
+
 static gboolean bail_out(GMainLoop* loop)
 {
     if (g_main_loop_is_running(loop))
@@ -742,6 +744,121 @@ static void testWebkitAtkTextAttributes(void)
     atk_attribute_set_free(set3);
 }
 
+static void testWekitAtkTextSelections(void)
+{
+    WebKitWebView* webView;
+    AtkObject* obj;
+    GMainLoop* loop;
+    gchar* selectedText;
+    gint startOffset;
+    gint endOffset;
+    gboolean result;
+
+    webView = WEBKIT_WEB_VIEW(webkit_web_view_new());
+    g_object_ref_sink(webView);
+    GtkAllocation alloc = { 0, 0, 800, 600 };
+    gtk_widget_size_allocate(GTK_WIDGET(webView), &alloc);
+    webkit_web_view_load_string(webView, textForSelections, NULL, NULL, NULL);
+    loop = g_main_loop_new(NULL, TRUE);
+
+    g_timeout_add(100, (GSourceFunc)bail_out, loop);
+    g_main_loop_run(loop);
+
+    obj = gtk_widget_get_accessible(GTK_WIDGET(webView));
+    g_assert(obj);
+
+    AtkText* paragraph1 = ATK_TEXT(atk_object_ref_accessible_child(obj, 0));
+    g_assert(ATK_IS_TEXT(paragraph1));
+    AtkText* paragraph2 = ATK_TEXT(atk_object_ref_accessible_child(obj, 1));
+    g_assert(ATK_IS_TEXT(paragraph2));
+    AtkText* link = ATK_TEXT(atk_object_ref_accessible_child(ATK_OBJECT(paragraph2), 0));
+    g_assert(ATK_IS_TEXT(link));
+
+    // First paragraph (simple text)
+
+    // Basic initial checks
+    g_assert_cmpint(atk_text_get_n_selections(paragraph1), ==, 0);
+    selectedText = atk_text_get_selection(paragraph1, 0, &startOffset, &endOffset);
+    g_assert_cmpint(startOffset, ==, 0);
+    g_assert_cmpint(endOffset, ==, 0);
+    g_assert_cmpstr(selectedText, ==, NULL);
+    g_free (selectedText);
+    // Try removing a non existing (yet) selection
+    result = atk_text_remove_selection(paragraph1, 0);
+    g_assert(!result);
+    // Try setting a 0-char selection
+    result = atk_text_set_selection(paragraph1, 0, 5, 5);
+    g_assert(result);
+
+    // Make a selection and test it
+    result = atk_text_set_selection(paragraph1, 0, 5, 25);
+    g_assert(result);
+    g_assert_cmpint(atk_text_get_n_selections(paragraph1), ==, 1);
+    selectedText = atk_text_get_selection(paragraph1, 0, &startOffset, &endOffset);
+    g_assert_cmpint(startOffset, ==, 5);
+    g_assert_cmpint(endOffset, ==, 25);
+    g_assert_cmpstr(selectedText, ==, "agraph with plain te");
+    g_free (selectedText);
+    // Try removing the selection from other AtkText object (should fail)
+    result = atk_text_remove_selection(paragraph2, 0);
+    g_assert(!result);
+
+    // Remove the selection and test everything again
+    result = atk_text_remove_selection(paragraph1, 0);
+    g_assert(result);
+    g_assert_cmpint(atk_text_get_n_selections(paragraph1), ==, 0);
+    selectedText = atk_text_get_selection(paragraph1, 0, &startOffset, &endOffset);
+    // Now offsets should be the same, set to the last position of the caret
+    g_assert_cmpint(startOffset, ==, endOffset);
+    g_assert_cmpint(startOffset, ==, 25);
+    g_assert_cmpint(endOffset, ==, 25);
+    g_assert_cmpstr(selectedText, ==, NULL);
+    g_free (selectedText);
+
+    // Second paragraph (text + link + text)
+
+    // Set a selection partially covering the link and test it
+    result = atk_text_set_selection(paragraph2, 0, 7, 21);
+    g_assert(result);
+
+    // Test the paragraph first
+    g_assert_cmpint(atk_text_get_n_selections(paragraph2), ==, 1);
+    selectedText = atk_text_get_selection(paragraph2, 0, &startOffset, &endOffset);
+    g_assert_cmpint(startOffset, ==, 7);
+    g_assert_cmpint(endOffset, ==, 21);
+    g_assert_cmpstr(selectedText, ==, "raph with a li");
+    g_free (selectedText);
+
+    // Now test just the link
+    g_assert_cmpint(atk_text_get_n_selections(link), ==, 1);
+    selectedText = atk_text_get_selection(link, 0, &startOffset, &endOffset);
+    g_assert_cmpint(startOffset, ==, 0);
+    g_assert_cmpint(endOffset, ==, 4);
+    g_assert_cmpstr(selectedText, ==, "a li");
+    g_free (selectedText);
+
+    // Remove selections and text everything again
+    result = atk_text_remove_selection(paragraph2, 0);
+    g_assert(result);
+    g_assert_cmpint(atk_text_get_n_selections(paragraph2), ==, 0);
+    selectedText = atk_text_get_selection(paragraph2, 0, &startOffset, &endOffset);
+    // Now offsets should be the same (no selection)
+    g_assert_cmpint(startOffset, ==, endOffset);
+    g_assert_cmpstr(selectedText, ==, NULL);
+    g_free (selectedText);
+
+    g_assert_cmpint(atk_text_get_n_selections(link), ==, 0);
+    selectedText = atk_text_get_selection(link, 0, &startOffset, &endOffset);
+    // Now offsets should be the same (no selection)
+    g_assert_cmpint(startOffset, ==, endOffset);
+    g_assert_cmpstr(selectedText, ==, NULL);
+    g_free (selectedText);
+
+    g_object_unref(paragraph1);
+    g_object_unref(paragraph2);
+    g_object_unref(webView);
+}
+
 static void test_webkit_atk_get_extents(void)
 {
     WebKitWebView* webView;
@@ -936,6 +1053,7 @@ int main(int argc, char** argv)
     g_test_add_func("/webkit/atk/getTextInTable", testWebkitAtkGetTextInTable);
     g_test_add_func("/webkit/atk/getHeadersInTable", testWebkitAtkGetHeadersInTable);
     g_test_add_func("/webkit/atk/textAttributes", testWebkitAtkTextAttributes);
+    g_test_add_func("/webkit/atk/textSelections", testWekitAtkTextSelections);
     g_test_add_func("/webkit/atk/get_extents", test_webkit_atk_get_extents);
     g_test_add_func("/webkit/atk/listsOfItems", testWebkitAtkListsOfItems);
     return g_test_run ();