Changeset 101349 in webkit


Ignore:
Timestamp:
Nov 29, 2011 3:04:43 AM (12 years ago)
Author:
mario@webkit.org
Message:

[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).

Location:
trunk/Source
Files:
13 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r101348 r101349  
     12011-11-29  Mario Sanchez Prada  <msanchez@igalia.com>
     2
     3        [Gtk] Regression: text-inserted events lack text inserted and current line
     4        https://bugs.webkit.org/show_bug.cgi?id=72830
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Replace the emission of the old (and now deprecated) AtkObject's
     9        'text-changed:insert' and 'text-changed:remove' signals with the
     10        new 'text-insert' and 'text-remove' ones, which are better and
     11        less fragile since they emit the modified text too, along with the
     12        typical 'offset' and 'count' values associated to the change.
     13
     14        Also, change the signature of the nodeTextChangeNotification() and
     15        nodeTextChangePlatformNotification() to allow specifying the text
     16        being modified from the place we better know about it, that is,
     17        the text editing commands.
     18
     19        * accessibility/gtk/AXObjectCacheAtk.cpp:
     20        (WebCore::emitTextChanged): Emit 'text-insert' and 'text-remove',
     21        instead of the old and now deprecated 'text-changed' signal.
     22        (WebCore::AXObjectCache::nodeTextChangePlatformNotification):
     23        Update this function to receive a String with the text being
     24        modified, instead of just the number of characters.
     25
     26        * accessibility/AXObjectCache.cpp:
     27        (WebCore::AXObjectCache::nodeTextChangeNotification): Update this
     28        function to receive a String with the text being modified.
     29        * accessibility/AXObjectCache.h:
     30        (WebCore::AXObjectCache::nodeTextChangeNotification): Ditto.
     31        (WebCore::AXObjectCache::nodeTextChangePlatformNotification): Ditto.
     32
     33        Adapt the text editing commants to pass the whole text string
     34        being modified, instead of just its number of characters.
     35
     36        * editing/AppendNodeCommand.cpp:
     37        (WebCore::sendAXTextChangedIgnoringLineBreaks): Adapt to the new
     38        signature of nodeTextChangeNotification(), so pass the whole text.
     39        * editing/DeleteFromTextNodeCommand.cpp:
     40        (WebCore::DeleteFromTextNodeCommand::doApply): Ditto.
     41        (WebCore::DeleteFromTextNodeCommand::doUnapply): Ditto.
     42        * editing/InsertIntoTextNodeCommand.cpp:
     43        (WebCore::InsertIntoTextNodeCommand::doApply): Ditto.
     44        (WebCore::InsertIntoTextNodeCommand::doUnapply): Ditto.
     45        * editing/InsertNodeBeforeCommand.cpp:
     46        (WebCore::InsertNodeBeforeCommand::doApply): Ditto.
     47        (WebCore::InsertNodeBeforeCommand::doUnapply): Ditto.
     48
     49        Update mac, win and chromium's specific parts of AXObjectCache to
     50        match the new signature for nodeTextChangePlatformNotification(),
     51        which won't affect their behaviour as they were not implementing
     52        that method anyway.
     53
     54        * accessibility/chromium/AXObjectCacheChromium.cpp:
     55        (WebCore::AXObjectCache::nodeTextChangePlatformNotification):
     56        * accessibility/mac/AXObjectCacheMac.mm:
     57        (WebCore::AXObjectCache::nodeTextChangePlatformNotification):
     58        * accessibility/win/AXObjectCacheWin.cpp:
     59        (WebCore::AXObjectCache::nodeTextChangePlatformNotification):
     60
    1612011-11-29  Mario Sanchez Prada  <msanchez@igalia.com>
    262

  • trunk/Source/WebCore/accessibility/AXObjectCache.cpp

    r97759 r101349  
    531531}
    532532
    533 void AXObjectCache::nodeTextChangeNotification(RenderObject* renderer, AXTextChange textChange, unsigned offset, unsigned count)
     533void AXObjectCache::nodeTextChangeNotification(RenderObject* renderer, AXTextChange textChange, unsigned offset, const String& text)
    534534{
    535535    if (!renderer)
    … …  
    538538    // Delegate on the right platform
    539539    AccessibilityObject* obj = getOrCreate(renderer);
    540     nodeTextChangePlatformNotification(obj, textChange, offset, count);
     540    nodeTextChangePlatformNotification(obj, textChange, offset, text);
    541541}
    542542#endif

  • trunk/Source/WebCore/accessibility/AXObjectCache.h

    r100057 r101349  
    147147    };
    148148
    149     void nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned offset, unsigned count);
     149    void nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned offset, const String&);
    150150
    151151    bool nodeHasRole(Node*, const AtomicString& role);
    … …  
    153153protected:
    154154    void postPlatformNotification(AccessibilityObject*, AXNotification);
    155     void nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned offset, unsigned count);
     155    void nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned offset, const String&);
    156156
    157157private:
    … …  
    186186inline void AXObjectCache::postNotification(AccessibilityObject*, Document*, AXNotification, bool postToElement, PostType) { }
    187187inline void AXObjectCache::postPlatformNotification(AccessibilityObject*, AXNotification) { }
    188 inline void AXObjectCache::nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned, unsigned) { }
    189 inline void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, unsigned) { }
     188inline void AXObjectCache::nodeTextChangeNotification(RenderObject*, AXTextChange, unsigned, const String&) { }
     189inline void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, const String&) { }
    190190inline void AXObjectCache::handleFocusedUIElementChanged(RenderObject*, RenderObject*) { }
    191191inline void AXObjectCache::handleScrolledToAnchor(const Node*) { }

  • trunk/Source/WebCore/accessibility/chromium/AXObjectCacheChromium.cpp

    r95901 r101349  
    104104}
    105105
    106 void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, unsigned)
     106void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, const String&)
    107107{
    108108}

  • trunk/Source/WebCore/accessibility/gtk/AXObjectCacheAtk.cpp

    r97539 r101349  
    159159}
    160160
    161 static void emitTextChanged(AccessibilityObject* object, AXObjectCache::AXTextChange textChange, unsigned offset, unsigned count)
    162 {
    163     // Get the axObject for the parent object
     161static void emitTextChanged(AccessibilityObject* object, AXObjectCache::AXTextChange textChange, unsigned offset, const String& text)
     162{
    164163    AtkObject* wrapper = object->parentObjectUnignored()->wrapper();
    165164    if (!wrapper || !ATK_IS_TEXT(wrapper))
    … …  
    170169    switch (textChange) {
    171170    case AXObjectCache::AXTextInserted:
    172         detail = "text-changed::insert";
     171        detail = "text-insert";
    173172        break;
    174173    case AXObjectCache::AXTextDeleted:
    175         detail = "text-changed::delete";
     174        detail = "text-remove";
    176175        break;
    177176    }
    178177
    179178    if (!detail.isNull())
    180         g_signal_emit_by_name(wrapper, detail.data(), offset, count);
    181 }
    182 
    183 void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject* object, AXTextChange textChange, unsigned offset, unsigned count)
     179        g_signal_emit_by_name(wrapper, detail.data(), offset, text.length(), text.utf8().data());
     180}
     181
     182void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject* object, AXTextChange textChange, unsigned offset, const String& text)
    184183{
    185184    // Sanity check
    186     if (count < 1 || !object || !object->isAccessibilityRenderObject())
     185    if (!object || !object->isAccessibilityRenderObject() || text.isEmpty())
    187186        return;
    188187
    189188    Node* node = object->node();
    190189    RefPtr<Range> range = Range::create(node->document(), node->parentNode(), 0, node, 0);
    191     emitTextChanged(object, textChange, offset + TextIterator::rangeLength(range.get()), count);
     190    emitTextChanged(object, textChange, offset + TextIterator::rangeLength(range.get()), text);
    192191}
    193192

  • trunk/Source/WebCore/accessibility/mac/AXObjectCacheMac.mm

    r95448 r101349  
    129129}
    130130
    131 void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, unsigned)
     131void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, const String&)
    132132{
    133133}

  • trunk/Source/WebCore/accessibility/win/AXObjectCacheWin.cpp

    r95901 r101349  
    108108}
    109109
    110 void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, unsigned)
     110void AXObjectCache::nodeTextChangePlatformNotification(AccessibilityObject*, AXTextChange, unsigned, const String&)
    111111{
    112112}

  • trunk/Source/WebCore/editing/AppendNodeCommand.cpp

    r86135 r101349  
    4848{
    4949    String nodeValue = node->nodeValue();
    50     unsigned len = nodeValue.length();
    5150    // Don't consider linebreaks in this command
    5251    if (nodeValue == "\n")
    5352      return;
    5453
    55     node->document()->axObjectCache()->nodeTextChangeNotification(node->renderer(), textChange, 0, len);
     54    node->document()->axObjectCache()->nodeTextChangeNotification(node->renderer(), textChange, 0, nodeValue);
    5655}
    5756

  • trunk/Source/WebCore/editing/DeleteFromTextNodeCommand.cpp

    r86135 r101349  
    5858    // Need to notify this before actually deleting the text
    5959    if (AXObjectCache::accessibilityEnabled())
    60         document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextDeleted, m_offset, m_count);
     60        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextDeleted, m_offset, m_text);
    6161
    6262    m_node->deleteData(m_offset, m_count, ec);
    … …  
    7474
    7575    if (AXObjectCache::accessibilityEnabled())
    76         document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextInserted, m_offset, m_count);
     76        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextInserted, m_offset, m_text);
    7777}
    7878

  • trunk/Source/WebCore/editing/InsertIntoTextNodeCommand.cpp

    r93656 r101349  
    6161
    6262    if (AXObjectCache::accessibilityEnabled())
    63         document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextInserted, m_offset, m_text.length());
     63        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextInserted, m_offset, m_text);
    6464}
    6565
    … …  
    7171    // Need to notify this before actually deleting the text
    7272    if (AXObjectCache::accessibilityEnabled())
    73         document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextDeleted, m_offset, m_text.length());
     73        document()->axObjectCache()->nodeTextChangeNotification(m_node->renderer(), AXObjectCache::AXTextDeleted, m_offset, m_text);
    7474
    7575    ExceptionCode ec;

  • trunk/Source/WebCore/editing/InsertNodeBeforeCommand.cpp

    r86135 r101349  
    5656
    5757    if (AXObjectCache::accessibilityEnabled())
    58         document()->axObjectCache()->nodeTextChangeNotification(m_insertChild->renderer(), AXObjectCache::AXTextInserted, 0, m_insertChild->nodeValue().length());
     58        document()->axObjectCache()->nodeTextChangeNotification(m_insertChild->renderer(), AXObjectCache::AXTextInserted, 0, m_insertChild->nodeValue());
    5959}
    6060
    … …  
    6666    // Need to notify this before actually deleting the text
    6767    if (AXObjectCache::accessibilityEnabled())
    68         document()->axObjectCache()->nodeTextChangeNotification(m_insertChild->renderer(), AXObjectCache::AXTextDeleted, 0, m_insertChild->nodeValue().length());
     68        document()->axObjectCache()->nodeTextChangeNotification(m_insertChild->renderer(), AXObjectCache::AXTextDeleted, 0, m_insertChild->nodeValue());
    6969
    7070    ExceptionCode ec;

  • trunk/Source/WebKit/gtk/ChangeLog

    r101271 r101349  
     12011-11-29  Mario Sanchez Prada  <msanchez@igalia.com>
     2
     3        [Gtk] Regression: text-inserted events lack text inserted and current line
     4        https://bugs.webkit.org/show_bug.cgi?id=72830
     5
     6        Reviewed by Chris Fleizach.
     7
     8        Updated unit test to handle the new 'text-insert' and
     9        'text-remove' signals, instead of the 'text-changed' one.
     10
     11        * tests/testatk.c:
     12        (textChangedCb): Update a global variable with the result of the
     13        text change, so we can check its value later.
     14        (testWebkitAtkTextChangedNotifications): Connect to the
     15        'text-insert' and 'text-remove' signals and check, in a way more
     16        carefully way than it was done before, that the signals are being
     17        properly emitted, and that the information attached to them is the
     18        right one for each case (insert/remove, offset, count and text).
     19
    1202011-11-28  Stefan Zwanenburg  <stefanhetzwaantje@gmail.com>
    221

  • trunk/Source/WebKit/gtk/tests/testatk.c

    r101271 r101349  
    15491549}
    15501550
    1551 static gboolean textInserted = FALSE;
    1552 static gboolean textDeleted = FALSE;
    1553 
    1554 static void textChangedCb(AtkText* text, gint pos, gint len, const gchar* detail)
     1551typedef enum {
     1552  TEXT_CHANGE_INSERT = 1,
     1553  TEXT_CHANGE_REMOVE = 2
     1554} TextChangeType;
     1555
     1556static gchar* textChangedResult = 0;
     1557
     1558static void textChangedCb(AtkText* text, gint pos, gint len, gchar* modifiedText, gpointer data)
    15551559{
    15561560    g_assert(text && ATK_IS_OBJECT(text));
    15571561
    1558     if (!g_strcmp0(detail, "insert"))
    1559         textInserted = TRUE;
    1560     else if (!g_strcmp0(detail, "delete"))
    1561         textDeleted = TRUE;
    1562 }
    1563 
    1564 static gboolean checkTextChanges(gpointer unused)
    1565 {
    1566     g_assert_cmpint(textInserted, ==, TRUE);
    1567     g_assert_cmpint(textDeleted, ==, TRUE);
    1568     return FALSE;
     1562    TextChangeType type = GPOINTER_TO_INT(data);
     1563    g_free(textChangedResult);
     1564    textChangedResult = g_strdup_printf("|%d|%d|%d|'%s'|", type, pos, len, modifiedText);
    15691565}
    15701566
    … …  
    15871583    g_assert(atk_object_get_role(ATK_OBJECT(textEntry)) == ATK_ROLE_ENTRY);
    15881584
    1589     g_signal_connect(textEntry, "text-changed::insert",
     1585    g_signal_connect(textEntry, "text-insert",
    15901586                     G_CALLBACK(textChangedCb),
    1591                      (gpointer)"insert");
    1592     g_signal_connect(textEntry, "text-changed::delete",
     1587                     GINT_TO_POINTER(TEXT_CHANGE_INSERT));
     1588    g_signal_connect(textEntry, "text-remove",
    15931589                     G_CALLBACK(textChangedCb),
    1594                      (gpointer)"delete");
     1590                     GINT_TO_POINTER(TEXT_CHANGE_REMOVE));
    15951591
    15961592    gint pos = 0;
    15971593    atk_editable_text_insert_text(ATK_EDITABLE_TEXT(textEntry), "foo bar baz", 11, &pos);
     1594    char* text = atk_text_get_text(ATK_TEXT(textEntry), 0, -1);
     1595    g_assert_cmpstr(text, ==, "foo bar baz");
     1596    g_assert_cmpstr(textChangedResult, ==, "|1|0|11|'foo bar baz'|");
     1597    g_free(text);
     1598
    15981599    atk_editable_text_delete_text(ATK_EDITABLE_TEXT(textEntry), 4, 7);
    1599     textInserted = FALSE;
    1600     textDeleted = FALSE;
    1601 
    1602     g_idle_add((GSourceFunc)checkTextChanges, 0);
     1600    text = atk_text_get_text(ATK_TEXT(textEntry), 0, -1);
     1601    g_assert_cmpstr(text, ==, "foo  baz");
     1602    g_assert_cmpstr(textChangedResult, ==, "|2|4|3|'bar'|");
     1603    g_free(text);
     1604
     1605    pos = 4;
     1606    atk_editable_text_insert_text(ATK_EDITABLE_TEXT(textEntry), "qux quux", 8, &pos);
     1607    text = atk_text_get_text(ATK_TEXT(textEntry), 0, -1);
     1608    g_assert_cmpstr(text, ==, "foo qux quux baz");
     1609    g_assert_cmpstr(textChangedResult, ==, "|1|4|8|'qux quux'|");
     1610    g_free(text);
     1611
     1612    g_free(textChangedResult);
    16031613
    16041614    g_object_unref(form);
Note: See TracChangeset for help on using the changeset viewer.