[Gtk] Regression: text-inserted events lack text inserted and current…

… line

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

Reviewed by Chris Fleizach.

Source/WebCore:

Replace the emission of the old (and now deprecated) AtkObject's
'text-changed:insert' and 'text-changed:remove' signals with the
new 'text-insert' and 'text-remove' ones, which are better and
less fragile since they emit the modified text too, along with the
typical 'offset' and 'count' values associated to the change.

Also, change the signature of the nodeTextChangeNotification() and
nodeTextChangePlatformNotification() to allow specifying the text
being modified from the place we better know about it, that is,
the text editing commands.

* accessibility/gtk/AXObjectCacheAtk.cpp:
(WebCore::emitTextChanged): Emit 'text-insert' and 'text-remove',
instead of the old and now deprecated 'text-changed' signal.
(WebCore::AXObjectCache::nodeTextChangePlatformNotification):
Update this function to receive a String with the text being
modified, instead of just the number of characters.

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::nodeTextChangeNotification): Update this
function to receive a String with the text being modified.
* accessibility/AXObjectCache.h:
(WebCore::AXObjectCache::nodeTextChangeNotification): Ditto.
(WebCore::AXObjectCache::nodeTextChangePlatformNotification): Ditto.

Adapt the text editing commants to pass the whole text string
being modified, instead of just its number of characters.

* editing/AppendNodeCommand.cpp:
(WebCore::sendAXTextChangedIgnoringLineBreaks): Adapt to the new
signature of nodeTextChangeNotification(), so pass the whole text.
* editing/DeleteFromTextNodeCommand.cpp:
(WebCore::DeleteFromTextNodeCommand::doApply): Ditto.
(WebCore::DeleteFromTextNodeCommand::doUnapply): Ditto.
* editing/InsertIntoTextNodeCommand.cpp:
(WebCore::InsertIntoTextNodeCommand::doApply): Ditto.
(WebCore::InsertIntoTextNodeCommand::doUnapply): Ditto.
* editing/InsertNodeBeforeCommand.cpp:
(WebCore::InsertNodeBeforeCommand::doApply): Ditto.
(WebCore::InsertNodeBeforeCommand::doUnapply): Ditto.

Update mac, win and chromium's specific parts of AXObjectCache to
match the new signature for nodeTextChangePlatformNotification(),
which won't affect their behaviour as they were not implementing
that method anyway.

* accessibility/chromium/AXObjectCacheChromium.cpp:
(WebCore::AXObjectCache::nodeTextChangePlatformNotification):
* accessibility/mac/AXObjectCacheMac.mm:
(WebCore::AXObjectCache::nodeTextChangePlatformNotification):
* accessibility/win/AXObjectCacheWin.cpp:
(WebCore::AXObjectCache::nodeTextChangePlatformNotification):

Source/WebKit/gtk:

Updated unit test to handle the new 'text-insert' and
'text-remove' signals, instead of the 'text-changed' one.

* tests/testatk.c:
(textChangedCb): Update a global variable with the result of the
text change, so we can check its value later.
(testWebkitAtkTextChangedNotifications): Connect to the
'text-insert' and 'text-remove' signals and check, in a way more
carefully way than it was done before, that the signals are being
properly emitted, and that the information attached to them is the
right one for each case (insert/remove, offset, count and text).

Canonical link: https://commits.webkit.org/89830@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@101349 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,63 @@
+2011-11-29  Mario Sanchez Prada  <msanchez@igalia.com>
+
+        [Gtk] Regression: text-inserted events lack text inserted and current line
+        https://bugs.webkit.org/show_bug.cgi?id=72830
+
+        Reviewed by Chris Fleizach.
+
+        Replace the emission of the old (and now deprecated) AtkObject's
+        'text-changed:insert' and 'text-changed:remove' signals with the
+        new 'text-insert' and 'text-remove' ones, which are better and
+        less fragile since they emit the modified text too, along with the
+        typical 'offset' and 'count' values associated to the change.
+
+        Also, change the signature of the nodeTextChangeNotification() and
+        nodeTextChangePlatformNotification() to allow specifying the text
+        being modified from the place we better know about it, that is,
+        the text editing commands.
+
+        * accessibility/gtk/AXObjectCacheAtk.cpp:
+        (WebCore::emitTextChanged): Emit 'text-insert' and 'text-remove',
+        instead of the old and now deprecated 'text-changed' signal.
+        (WebCore::AXObjectCache::nodeTextChangePlatformNotification):
+        Update this function to receive a String with the text being
+        modified, instead of just the number of characters.
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::nodeTextChangeNotification): Update this
+        function to receive a String with the text being modified.
+        * accessibility/AXObjectCache.h:
+        (WebCore::AXObjectCache::nodeTextChangeNotification): Ditto.
+        (WebCore::AXObjectCache::nodeTextChangePlatformNotification): Ditto.
+
+        Adapt the text editing commants to pass the whole text string
+        being modified, instead of just its number of characters.
+
+        * editing/AppendNodeCommand.cpp:
+        (WebCore::sendAXTextChangedIgnoringLineBreaks): Adapt to the new
+        signature of nodeTextChangeNotification(), so pass the whole text.
+        * editing/DeleteFromTextNodeCommand.cpp:
+        (WebCore::DeleteFromTextNodeCommand::doApply): Ditto.
+        (WebCore::DeleteFromTextNodeCommand::doUnapply): Ditto.
+        * editing/InsertIntoTextNodeCommand.cpp:
+        (WebCore::InsertIntoTextNodeCommand::doApply): Ditto.
+        (WebCore::InsertIntoTextNodeCommand::doUnapply): Ditto.
+        * editing/InsertNodeBeforeCommand.cpp:
+        (WebCore::InsertNodeBeforeCommand::doApply): Ditto.
+        (WebCore::InsertNodeBeforeCommand::doUnapply): Ditto.
+
+        Update mac, win and chromium's specific parts of AXObjectCache to
+        match the new signature for nodeTextChangePlatformNotification(),
+        which won't affect their behaviour as they were not implementing
+        that method anyway.
+
+        * accessibility/chromium/AXObjectCacheChromium.cpp:
+        (WebCore::AXObjectCache::nodeTextChangePlatformNotification):
+        * accessibility/mac/AXObjectCacheMac.mm:
+        (WebCore::AXObjectCache::nodeTextChangePlatformNotification):
+        * accessibility/win/AXObjectCacheWin.cpp:
+        (WebCore::AXObjectCache::nodeTextChangePlatformNotification):
+
 2011-11-29  Mario Sanchez Prada  <msanchez@igalia.com>
 
         [Gtk] Regression: Push buttons no longer expose their displayed text/name

Source/WebCore/accessibility/AXObjectCache.cpp

@@ -530,14 +530,14 @@ void AXObjectCache::selectedChildrenChanged(RenderObject* renderer)
     postNotification(renderer, AXSelectedChildrenChanged, false);
 }
 
-void AXObjectCache::nodeTextChangeNotification(RenderObject* renderer, AXTextChange textChange, unsigned offset, unsigned count)
+void AXObjectCache::nodeTextChangeNotification(RenderObject* renderer, AXTextChange textChange, unsigned offset, const String& text)
 {
     if (!renderer)
         return;
 
     // Delegate on the right platform
     AccessibilityObject* obj = getOrCreate(renderer);
-    nodeTextChangePlatformNotification(obj, textChange, offset, count);
+    nodeTextChangePlatformNotification(obj, textChange, offset, text);
 }
 #endif
 

Source/WebCore/accessibility/AXObjectCache.h

@@ -146,13 +146,13 @@ class AXObjectCache {
         AXTextDeleted,
     };
 
-    void nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned offset, unsigned count);
+    void nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned offset, const String&);
 
     bool nodeHasRole(Node*, const AtomicString& role);
 
 protected:
     void postPlatformNotification(AccessibilityObject*, AXNotification);
-    void nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned offset, unsigned count);
+    void nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned offset, const String&);
 
 private:
     Document* m_document;
@@ -185,8 +185,8 @@ inline void AXObjectCache::selectedChildrenChanged(RenderObject*) { }
 inline void AXObjectCache::postNotification(RenderObject*, AXNotification, bool postToElement, PostType) { }
 inline void AXObjectCache::postNotification(AccessibilityObject*, Document*, AXNotification, bool postToElement, PostType) { }
 inline void AXObjectCache::postPlatformNotification(AccessibilityObject*, AXNotification) { }
-inline void AXObjectCache::nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned, unsigned) { }
-inline void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, unsigned) { }
+inline void AXObjectCache::nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned, const String&) { }
+inline void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, const String&) { }
 inline void AXObjectCache::handleFocusedUIElementChanged(RenderObject*, RenderObject*) { }
 inline void AXObjectCache::handleScrolledToAnchor(const Node*) { }
 inline void AXObjectCache::contentChanged(RenderObject*) { }

Source/WebCore/accessibility/chromium/AXObjectCacheChromium.cpp

@@ -103,7 +103,7 @@ void AXObjectCache::postPlatformNotification(AccessibilityObject* obj, AXNotific
     client->postAccessibilityNotification(obj, notification);        
 }
 
-void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, unsigned)
+void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, const String&)
 {
 }
 

Source/WebCore/accessibility/gtk/AXObjectCacheAtk.cpp

@@ -158,9 +158,8 @@ void AXObjectCache::postPlatformNotification(AccessibilityObject* coreObject, AX
     }
 }
 
-static void emitTextChanged(AccessibilityObject* object, AXObjectCache::AXTextChange textChange, unsigned offset, unsigned count)
+static void emitTextChanged(AccessibilityObject* object, AXObjectCache::AXTextChange textChange, unsigned offset, const String& text)
 {
-    // Get the axObject for the parent object
     AtkObject* wrapper = object->parentObjectUnignored()->wrapper();
     if (!wrapper || !ATK_IS_TEXT(wrapper))
         return;
@@ -169,26 +168,26 @@ static void emitTextChanged(AccessibilityObject* object, AXObjectCache::AXTextCh
     CString detail;
     switch (textChange) {
     case AXObjectCache::AXTextInserted:
-        detail = "text-changed::insert";
+        detail = "text-insert";
         break;
     case AXObjectCache::AXTextDeleted:
-        detail = "text-changed::delete";
+        detail = "text-remove";
         break;
     }
 
     if (!detail.isNull())
-        g_signal_emit_by_name(wrapper, detail.data(), offset, count);
+        g_signal_emit_by_name(wrapper, detail.data(), offset, text.length(), text.utf8().data());
 }
 
-void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject* object, AXTextChange textChange, unsigned offset, unsigned count)
+void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject* object, AXTextChange textChange, unsigned offset, const String& text)
 {
     // Sanity check
-    if (count < 1 || !object || !object->isAccessibilityRenderObject())
+    if (!object || !object->isAccessibilityRenderObject() || text.isEmpty())
         return;
 
     Node* node = object->node();
     RefPtr<Range> range = Range::create(node->document(), node->parentNode(), 0, node, 0);
-    emitTextChanged(object, textChange, offset + TextIterator::rangeLength(range.get()), count);
+    emitTextChanged(object, textChange, offset + TextIterator::rangeLength(range.get()), text);
 }
 
 void AXObjectCache::handleFocusedUIElementChanged(RenderObject* oldFocusedRender, RenderObject* newFocusedRender)

Source/WebCore/accessibility/mac/AXObjectCacheMac.mm

@@ -128,7 +128,7 @@
     [obj->wrapper() accessibilityPostedNotification:macNotification];
 }
 
-void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, unsigned)
+void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, const String&)
 {
 }
 

Source/WebCore/accessibility/win/AXObjectCacheWin.cpp

@@ -107,7 +107,7 @@ void AXObjectCache::postPlatformNotification(AccessibilityObject* obj, AXNotific
     NotifyWinEvent(msaaEvent, page->chrome()->platformPageClient(), OBJID_CLIENT, -static_cast<LONG>(obj->axObjectID()));
 }
 
-void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, unsigned)
+void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, const String&)
 {
 }
 

Source/WebCore/editing/AppendNodeCommand.cpp

@@ -47,12 +47,11 @@ AppendNodeCommand::AppendNodeCommand(PassRefPtr<ContainerNode> parent, PassRefPt
 static void sendAXTextChangedIgnoringLineBreaks(Node* node, AXObjectCache::AXTextChange textChange)
 {
     String nodeValue = node->nodeValue();
-    unsigned len = nodeValue.length();
     // Don't consider linebreaks in this command
     if (nodeValue == "\n")
       return;
 
-    node->document()->axObjectCache()->nodeTextChangeNotification(node->renderer(), textChange, 0, len);
+    node->document()->axObjectCache()->nodeTextChangeNotification(node->renderer(), textChange, 0, nodeValue);
 }
 
 void AppendNodeCommand::doApply()

Source/WebCore/editing/DeleteFromTextNodeCommand.cpp

@@ -57,7 +57,7 @@ void DeleteFromTextNodeCommand::doApply()
 
     // Need to notify this before actually deleting the text
     if (AXObjectCache::accessibilityEnabled())
-        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextDeleted, m_offset, m_count);
+        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextDeleted, m_offset, m_text);
 
     m_node->deleteData(m_offset, m_count, ec);
 }
@@ -73,7 +73,7 @@ void DeleteFromTextNodeCommand::doUnapply()
     m_node->insertData(m_offset, m_text, ec);
 
     if (AXObjectCache::accessibilityEnabled())
-        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextInserted, m_offset, m_count);
+        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextInserted, m_offset, m_text);
 }
 
 } // namespace WebCore

Source/WebCore/editing/InsertIntoTextNodeCommand.cpp

@@ -60,7 +60,7 @@ void InsertIntoTextNodeCommand::doApply()
     m_node->insertData(m_offset, m_text, ec);
 
     if (AXObjectCache::accessibilityEnabled())
-        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextInserted, m_offset, m_text.length());
+        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextInserted, m_offset, m_text);
 }
 
 void InsertIntoTextNodeCommand::doUnapply()
@@ -70,7 +70,7 @@ void InsertIntoTextNodeCommand::doUnapply()
 
     // Need to notify this before actually deleting the text
     if (AXObjectCache::accessibilityEnabled())
-        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextDeleted, m_offset, m_text.length());
+        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextDeleted, m_offset, m_text);
 
     ExceptionCode ec;
     m_node->deleteData(m_offset, m_text.length(), ec);

Source/WebCore/editing/InsertNodeBeforeCommand.cpp

@@ -55,7 +55,7 @@ void InsertNodeBeforeCommand::doApply()
     parent->insertBefore(m_insertChild.get(), m_refChild.get(), ec);
 
     if (AXObjectCache::accessibilityEnabled())
-        document()->axObjectCache()->nodeTextChangeNotification(m_insertChild->renderer(), AXObjectCache::AXTextInserted, 0, m_insertChild->nodeValue().length());
+        document()->axObjectCache()->nodeTextChangeNotification(m_insertChild->renderer(), AXObjectCache::AXTextInserted, 0, m_insertChild->nodeValue());
 }
 
 void InsertNodeBeforeCommand::doUnapply()
@@ -65,7 +65,7 @@ void InsertNodeBeforeCommand::doUnapply()
 
     // Need to notify this before actually deleting the text
     if (AXObjectCache::accessibilityEnabled())
-        document()->axObjectCache()->nodeTextChangeNotification(m_insertChild->renderer(), AXObjectCache::AXTextDeleted, 0, m_insertChild->nodeValue().length());
+        document()->axObjectCache()->nodeTextChangeNotification(m_insertChild->renderer(), AXObjectCache::AXTextDeleted, 0, m_insertChild->nodeValue());
 
     ExceptionCode ec;
     m_insertChild->remove(ec);

Source/WebKit/gtk/ChangeLog

@@ -1,3 +1,22 @@
+2011-11-29  Mario Sanchez Prada  <msanchez@igalia.com>
+
+        [Gtk] Regression: text-inserted events lack text inserted and current line
+        https://bugs.webkit.org/show_bug.cgi?id=72830
+
+        Reviewed by Chris Fleizach.
+
+        Updated unit test to handle the new 'text-insert' and
+        'text-remove' signals, instead of the 'text-changed' one.
+
+        * tests/testatk.c:
+        (textChangedCb): Update a global variable with the result of the
+        text change, so we can check its value later.
+        (testWebkitAtkTextChangedNotifications): Connect to the
+        'text-insert' and 'text-remove' signals and check, in a way more
+        carefully way than it was done before, that the signals are being
+        properly emitted, and that the information attached to them is the
+        right one for each case (insert/remove, offset, count and text).
+
 2011-11-28  Stefan Zwanenburg  <stefanhetzwaantje@gmail.com>
 
         WebKitGTK+-1.7.2 build error due to a reference to gdk_disable_multidevice()

Source/WebKit/gtk/tests/testatk.c

@@ -1548,24 +1548,20 @@ static void testWebkitAtkListsOfItems()
     g_object_unref(webView);
 }
 
-static gboolean textInserted = FALSE;
-static gboolean textDeleted = FALSE;
+typedef enum {
+  TEXT_CHANGE_INSERT = 1,
+  TEXT_CHANGE_REMOVE = 2
+} TextChangeType;
 
-static void textChangedCb(AtkText* text, gint pos, gint len, const gchar* detail)
+static gchar* textChangedResult = 0;
+
+static void textChangedCb(AtkText* text, gint pos, gint len, gchar* modifiedText, gpointer data)
 {
     g_assert(text && ATK_IS_OBJECT(text));
 
-    if (!g_strcmp0(detail, "insert"))
-        textInserted = TRUE;
-    else if (!g_strcmp0(detail, "delete"))
-        textDeleted = TRUE;
-}
-
-static gboolean checkTextChanges(gpointer unused)
-{
-    g_assert_cmpint(textInserted, ==, TRUE);
-    g_assert_cmpint(textDeleted, ==, TRUE);
-    return FALSE;
+    TextChangeType type = GPOINTER_TO_INT(data);
+    g_free(textChangedResult);
+    textChangedResult = g_strdup_printf("|%d|%d|%d|'%s'|", type, pos, len, modifiedText);
 }
 
 static void testWebkitAtkTextChangedNotifications()
@@ -1586,20 +1582,34 @@ static void testWebkitAtkTextChangedNotifications()
     g_assert(ATK_IS_EDITABLE_TEXT(textEntry));
     g_assert(atk_object_get_role(ATK_OBJECT(textEntry)) == ATK_ROLE_ENTRY);
 
-    g_signal_connect(textEntry, "text-changed::insert",
+    g_signal_connect(textEntry, "text-insert",
                      G_CALLBACK(textChangedCb),
-                     (gpointer)"insert");
-    g_signal_connect(textEntry, "text-changed::delete",
+                     GINT_TO_POINTER(TEXT_CHANGE_INSERT));
+    g_signal_connect(textEntry, "text-remove",
                      G_CALLBACK(textChangedCb),
-                     (gpointer)"delete");
+                     GINT_TO_POINTER(TEXT_CHANGE_REMOVE));
 
     gint pos = 0;
     atk_editable_text_insert_text(ATK_EDITABLE_TEXT(textEntry), "foo bar baz", 11, &pos);
+    char* text = atk_text_get_text(ATK_TEXT(textEntry), 0, -1);
+    g_assert_cmpstr(text, ==, "foo bar baz");
+    g_assert_cmpstr(textChangedResult, ==, "|1|0|11|'foo bar baz'|");
+    g_free(text);
+
     atk_editable_text_delete_text(ATK_EDITABLE_TEXT(textEntry), 4, 7);
-    textInserted = FALSE;
-    textDeleted = FALSE;
+    text = atk_text_get_text(ATK_TEXT(textEntry), 0, -1);
+    g_assert_cmpstr(text, ==, "foo  baz");
+    g_assert_cmpstr(textChangedResult, ==, "|2|4|3|'bar'|");
+    g_free(text);
+
+    pos = 4;
+    atk_editable_text_insert_text(ATK_EDITABLE_TEXT(textEntry), "qux quux", 8, &pos);
+    text = atk_text_get_text(ATK_TEXT(textEntry), 0, -1);
+    g_assert_cmpstr(text, ==, "foo qux quux baz");
+    g_assert_cmpstr(textChangedResult, ==, "|1|4|8|'qux quux'|");
+    g_free(text);
 
-    g_idle_add((GSourceFunc)checkTextChanges, 0);
+    g_free(textChangedResult);
 
     g_object_unref(form);
     g_object_unref(textEntry);