Title: [153427] trunk/Source
Revision
153427
Author
ma...@webkit.org
Date
2013-07-29 08:46:35 -0700 (Mon, 29 Jul 2013)

Log Message

[ATK] Issues with edge cases when getting offsets for a text range in AtkText
https://bugs.webkit.org/show_bug.cgi?id=118908

Reviewed by Martin Robinson.

Source/WebCore:

Reimplement getSelectionOffsetsForObject() just in term of
Positions instead of using ranges, which makes it simpler and
works better. Also, make sure we use the right Node as reference
for the accessibility object by getting the proper one both for
text control objects (e.g. input, text area) and normal ones.

* accessibility/atk/WebKitAccessibleInterfaceText.cpp:
(getNodeForAccessibilityObject): New helper function to ensure we
always get the relevant object from the DOM tree for a given
accessibility object, in the context of the implementation of
AtkText, so it works both with Text Controls and other elements.
(getSelectionOffsetsForObject): Rewritten this function in terms
of VisiblePositions and avoiding weird operations with ranges, so
we have more control to fine tune the results and give more
accurate ones. Besides, now it works better with edge cases.

Source/WebKit/gtk:

* tests/testatk.c:
(runGetTextTests): Updated unit tests to check more cases of
calling the atk_text_get_text_*_offset() functions for WORD.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (153426 => 153427)


--- trunk/Source/WebCore/ChangeLog	2013-07-29 15:36:54 UTC (rev 153426)
+++ trunk/Source/WebCore/ChangeLog	2013-07-29 15:46:35 UTC (rev 153427)
@@ -1,3 +1,26 @@
+2013-07-29  Mario Sanchez Prada  <mario.pr...@samsung.com>
+
+        [ATK] Issues with edge cases when getting offsets for a text range in AtkText
+        https://bugs.webkit.org/show_bug.cgi?id=118908
+
+        Reviewed by Martin Robinson.
+
+        Reimplement getSelectionOffsetsForObject() just in term of
+        Positions instead of using ranges, which makes it simpler and
+        works better. Also, make sure we use the right Node as reference
+        for the accessibility object by getting the proper one both for
+        text control objects (e.g. input, text area) and normal ones.
+
+        * accessibility/atk/WebKitAccessibleInterfaceText.cpp:
+        (getNodeForAccessibilityObject): New helper function to ensure we
+        always get the relevant object from the DOM tree for a given
+        accessibility object, in the context of the implementation of
+        AtkText, so it works both with Text Controls and other elements.
+        (getSelectionOffsetsForObject): Rewritten this function in terms
+        of VisiblePositions and avoiding weird operations with ranges, so
+        we have more control to fine tune the results and give more
+        accurate ones. Besides, now it works better with edge cases.
+
 2013-07-29  Zan Dobersek  <zdober...@igalia.com>
 
         Clean up DragImage

Modified: trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp (153426 => 153427)


--- trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp	2013-07-29 15:36:54 UTC (rev 153426)
+++ trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp	2013-07-29 15:46:35 UTC (rev 153427)
@@ -501,53 +501,65 @@
     return offset - offsetAdjustmentForListItem(coreObject);
 }
 
+static Node* getNodeForAccessibilityObject(AccessibilityObject* coreObject)
+{
+    if (!coreObject->isNativeTextControl())
+        return coreObject->node();
+
+    // For text controls, we get the first visible position on it (which will
+    // belong to its inner element, unreachable from the DOM) and return its
+    // parent node, so we have a "bounding node" for the accessibility object.
+    VisiblePosition positionInTextControlInnerElement = coreObject->visiblePositionForIndex(0, true);
+    Node* innerMostNode = positionInTextControlInnerElement.deepEquivalent().anchorNode();
+    if (!innerMostNode)
+        return 0;
+
+    return innerMostNode->parentNode();
+}
+
 static void getSelectionOffsetsForObject(AccessibilityObject* coreObject, VisibleSelection& selection, gint& startOffset, gint& endOffset)
 {
-    if (!coreObject->isAccessibilityRenderObject())
+    // Default values, unless the contrary is proved.
+    startOffset = 0;
+    endOffset = 0;
+
+    Node* node = getNodeForAccessibilityObject(coreObject);
+    if (!node)
         return;
 
-    // Early return if the selection doesn't affect the selected node.
-    if (!selectionBelongsToObject(coreObject, selection))
+    if (selection.isNone())
         return;
 
-    // We need to find the exact start and end positions in the
-    // selected node that intersects the selection, to later on get
-    // the right values for the effective start and end offsets.
-    Position nodeRangeStart;
-    Position nodeRangeEnd;
-    Node* node = coreObject->node();
-    RefPtr<Range> selRange = selection.toNormalizedRange();
+    // We need to limit our search to positions that fall inside the domain of the current object.
+    Position firstValidPosition = firstPositionInOrBeforeNode(node->firstDescendant());
+    Position lastValidPosition = lastPositionInOrAfterNode(node->lastDescendant());
 
-    // If the selection affects the selected node and its first
-    // possible position is also in the selection, we must set
-    // nodeRangeStart to that position, otherwise to the selection's
-    // start position (it would belong to the node anyway).
-    Node* firstLeafNode = node->firstDescendant();
-    if (selRange->isPointInRange(firstLeafNode, 0, IGNORE_EXCEPTION))
-        nodeRangeStart = firstPositionInOrBeforeNode(firstLeafNode);
-    else
-        nodeRangeStart = selRange->startPosition();
+    // Early return with proper values if the selection falls entirely out of the object.
+    if (!selectionBelongsToObject(coreObject, selection)) {
+        startOffset = comparePositions(selection.start(), firstValidPosition) < 0 ? 0 : accessibilityObjectLength(coreObject);
+        endOffset = startOffset;
+        return;
+    }
 
-    // If the selection affects the selected node and its last
-    // possible position is also in the selection, we must set
-    // nodeRangeEnd to that position, otherwise to the selection's
-    // end position (it would belong to the node anyway).
-    Node* lastLeafNode = node->lastDescendant();
-    if (selRange->isPointInRange(lastLeafNode, lastOffsetInNode(lastLeafNode), IGNORE_EXCEPTION))
-        nodeRangeEnd = lastPositionInOrAfterNode(lastLeafNode);
-    else
-        nodeRangeEnd = selRange->endPosition();
+    // Find the proper range for the selection that falls inside the object.
+    Position nodeRangeStart = selection.start();
+    if (comparePositions(nodeRangeStart, firstValidPosition) < 0)
+        nodeRangeStart = firstValidPosition;
 
+    Position nodeRangeEnd = selection.end();
+    if (comparePositions(nodeRangeEnd, lastValidPosition) > 0)
+        nodeRangeEnd = lastValidPosition;
+
     // Calculate position of the selected range inside the object.
     Position parentFirstPosition = firstPositionInOrBeforeNode(node);
     RefPtr<Range> rangeInParent = Range::create(node->document(), parentFirstPosition, nodeRangeStart);
 
-
-    // Set values for start and end offsets.
+    // Set values for start offsets and calculate initial range length.
+    // These values might be adjusted later to cover special cases.
     startOffset = webCoreOffsetToAtkOffset(coreObject, TextIterator::rangeLength(rangeInParent.get(), true));
-
     RefPtr<Range> nodeRange = Range::create(node->document(), nodeRangeStart, nodeRangeEnd);
-    endOffset = startOffset + TextIterator::rangeLength(nodeRange.get(), true);
+    int rangeLength = TextIterator::rangeLength(nodeRange.get(), true);
+    endOffset = std::min(startOffset + rangeLength, static_cast<int>(accessibilityObjectLength(coreObject)));
 }
 
 static gchar* webkitAccessibleTextGetText(AtkText* text, gint startOffset, gint endOffset)

Modified: trunk/Source/WebKit/gtk/ChangeLog (153426 => 153427)


--- trunk/Source/WebKit/gtk/ChangeLog	2013-07-29 15:36:54 UTC (rev 153426)
+++ trunk/Source/WebKit/gtk/ChangeLog	2013-07-29 15:46:35 UTC (rev 153427)
@@ -1,3 +1,14 @@
+2013-07-29  Mario Sanchez Prada  <mario.pr...@samsung.com>
+
+        [ATK] Issues with edge cases when getting offsets for a text range in AtkText
+        https://bugs.webkit.org/show_bug.cgi?id=118908
+
+        Reviewed by Martin Robinson.
+
+        * tests/testatk.c:
+        (runGetTextTests): Updated unit tests to check more cases of
+        calling the atk_text_get_text_*_offset() functions for WORD.
+
 2013-07-25  Andreas Kling  <akl...@apple.com>
 
         ChromeClient::focusedNodeChanged() should be focusedElementChanged().

Modified: trunk/Source/WebKit/gtk/tests/testatk.c (153426 => 153427)


--- trunk/Source/WebKit/gtk/tests/testatk.c	2013-07-29 15:36:54 UTC (rev 153426)
+++ trunk/Source/WebKit/gtk/tests/testatk.c	2013-07-29 15:46:35 UTC (rev 153427)
@@ -138,12 +138,21 @@
     testGetTextFunction(textObject, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_WORD_START,
                         58, "third.", 58, 64);
 
+    testGetTextFunction(textObject, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_WORD_START,
+                        64, "third.", 58, 64);
+
     testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_START,
+                        0, "", 0, 0);
+
+    testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_START,
                         5, "This ", 0, 5);
 
     testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_START,
                         7, "This ", 0, 5);
 
+    testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_START,
+                        64, "the ", 54, 58);
+
     testGetTextFunction(textObject, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_WORD_START,
                         0, "is ", 5, 8);
 
@@ -153,6 +162,9 @@
     testGetTextFunction(textObject, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_WORD_START,
                         3, "is ", 5, 8);
 
+    testGetTextFunction(textObject, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_WORD_START,
+                        64, "", 64, 64);
+
     /* ATK_TEXT_BOUNDARY_WORD_END */
     testGetTextFunction(textObject, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_WORD_END,
                         0, "This", 0, 4);
@@ -166,7 +178,16 @@
     testGetTextFunction(textObject, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_WORD_END,
                         9, " test", 9, 14);
 
+    testGetTextFunction(textObject, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_WORD_END,
+                        58, " third", 57, 63);
+
+    testGetTextFunction(textObject, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_WORD_END,
+                        64, ".", 63, 64);
+
     testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_END,
+                        0, "", 0, 0);
+
+    testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_END,
                         5, "This", 0, 4);
 
     testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_END,
@@ -175,14 +196,20 @@
     testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_END,
                         7, " is", 4, 7);
 
+    testGetTextFunction(textObject, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_WORD_END,
+                        64, " third", 57, 63);
+
     testGetTextFunction(textObject, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_WORD_END,
+                        0, " is", 4, 7);
+
+    testGetTextFunction(textObject, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_WORD_END,
                         5, " a", 7, 9);
 
     testGetTextFunction(textObject, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_WORD_END,
                         4, " a", 7, 9);
 
-    testGetTextFunction(textObject, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_WORD_END,
-                        58, " third", 57, 63);
+    testGetTextFunction(textObject, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_WORD_END,
+                        64, "", 64, 64);
 
     /* ATK_TEXT_BOUNDARY_SENTENCE_START */
     testGetTextFunction(textObject, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_SENTENCE_START,
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to