Changeset 67320 in webkit

Timestamp:

Sep 12, 2010 4:16:14 AM (14 years ago)

Author:

commit-queue@webkit.org

Message:

2010-09-12 Mario Sanchez Prada <​msanchez@igalia.com>

Reviewed by Martin Robinson.

[Gtk] get_n_selections and get_selection fail when selecting text across object boundaries
​https://bugs.webkit.org/show_bug.cgi?id=26991

Fix AtkText getNSelections() and getSelection() to work properly

• accessibility/gtk/AccessibilityObjectWrapperAtk.cpp: (selectionBelongsToObject): Check that both the selection intersects the node AND that the selection is not just "touching" one of the boundaries for the selected node. We want to check whether the node is actually inside the region, at least partially (getSelectionOffsetsForObject): New function to get the start and end offsets of a selection for a given accessible object. (webkit_accessible_text_get_selection): Return zero when both start and end offsets are equal, following the lead of GAIL.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• accessibility/gtk/AccessibilityObjectWrapperAtk.cpp (modified) (4 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-09-12  Mario Sanchez Prada  <msanchez@igalia.com>
+
+        Reviewed by Martin Robinson.
+
+        [Gtk] get_n_selections and get_selection fail when selecting text across object boundaries
+        https://bugs.webkit.org/show_bug.cgi?id=26991
+
+        Fix AtkText getNSelections() and getSelection() to work properly
+
+        * accessibility/gtk/AccessibilityObjectWrapperAtk.cpp:
+        (selectionBelongsToObject): Check that both the selection intersects
+        the node AND that the selection is not just "touching" one of the
+        boundaries for the selected node. We want to check whether the
+        node is actually inside the region, at least partially
+        (getSelectionOffsetsForObject): New function to get the start and
+        end offsets of a selection for a given accessible object.
+        (webkit_accessible_text_get_selection): Return zero when both
+        start and end offsets are equal, following the lead of GAIL.
+
 2010-09-11  Adam Barth  <abarth@webkit.org>
 

trunk/WebCore/accessibility/gtk/AccessibilityObjectWrapperAtk.cpp

@@ -59 +59 @@
 #include "RenderText.h"
 #include "TextEncoding.h"
+#include "TextIterator.h"
 #include <wtf/text/CString.h>
 #include <wtf/text/AtomicString.h>
@@ -1343 +1344 @@
         return false;
 
-    Node* node = static_cast<AccessibilityRenderObject*>(coreObject)->renderer()->node();
-    return node == selection.base().containerNode();
+    RefPtr<Range> range = selection.toNormalizedRange();
+    if (!range)
+        return false;
+
+    // We want to check that both the selection intersects the node
+    // AND that the selection is not just "touching" one of the
+    // boundaries for the selected node. We want to check whether the
+    // node is actually inside the region, at least partially
+    Node* node = coreObject->node();
+    Node* lastDescendant = node->lastDescendant();
+    ExceptionCode ec = 0;
+    return (range->intersectsNode(node, ec)
+            && (range->endContainer() != node || range->endOffset())
+            && (range->startContainer() != lastDescendant || range->startOffset() != lastOffsetInNode(lastDescendant)));
+}
+
+static void getSelectionOffsetsForObject(AccessibilityObject* coreObject, VisibleSelection& selection, gint& startOffset, gint& endOffset)
+{
+    if (!coreObject->isAccessibilityRenderObject())
+        return;
+
+    // Early return if the selection doesn't affect the selected node
+    if (!selectionBelongsToObject(coreObject, selection))
+        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
+    ExceptionCode ec = 0;
+    Position nodeRangeStart;
+    Position nodeRangeEnd;
+    Node* node = coreObject->node();
+    RefPtr<Range> selRange = selection.toNormalizedRange();
+
+    // 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, ec))
+        nodeRangeStart = firstPositionInNode(firstLeafNode);
+    else
+        nodeRangeStart = selRange->startPosition();
+
+    // 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), ec))
+        nodeRangeEnd = lastPositionInNode(lastLeafNode);
+    else
+        nodeRangeEnd = selRange->endPosition();
+
+    // Set preliminar values for start and end offsets
+    startOffset = nodeRangeStart.offsetInContainerNode();
+    endOffset = nodeRangeEnd.offsetInContainerNode();
+
+    // If the end node is different then the start node, iterate over
+    // those among them to build the effective value for endOffset
+    if (nodeRangeStart.anchorNode() != nodeRangeEnd.anchorNode()) {
+        RefPtr<Range> nodeRange = Range::create(node->document(), nodeRangeStart, positionBeforeNode(nodeRangeEnd.anchorNode()));
+        for (TextIterator it(nodeRange.get()); !it.atEnd(); it.advance()) {
+            RefPtr<Range> range = it.range();
+            if (range->startContainer()->isTextNode())
+                endOffset += range->endOffset();
+        }
+    }
 }
 
@@ -1351 +1418 @@
     AccessibilityObject* coreObject = core(text);
     VisibleSelection selection = coreObject->selection();
+
+    // Only range selections are needed for the purpose of this method
+    if (!selection.isRange())
+        return 0;
 
     // We don't support multiple selections for now, so there's only
@@ -1361 +1432 @@
 }
 
-static gchar* webkit_accessible_text_get_selection(AtkText* text, gint selection_num, gint* start_offset, gint* end_offset)
-{
+static gchar* webkit_accessible_text_get_selection(AtkText* text, gint selectionNum, gint* startOffset, gint* endOffset)
+{
+    // Default values, unless the contrary is proved
+    *startOffset = *endOffset = 0;
+
+    // WebCore does not support multiple selection, so anything but 0 does not make sense for now.
+    if (selectionNum)
+        return 0;
+
+    // Get the offsets of the selection for the selected object
     AccessibilityObject* coreObject = core(text);
     VisibleSelection selection = coreObject->selection();
-
-    // WebCore does not support multiple selection, so anything but 0 does not make sense for now.
-    // Also, we don't want to do anything if the selection does not
-    // belong to the currently selected object. We have to check since
-    // there's no way to get the selection for a given object, only
-    // the global one (the API is a bit confusing)
-    if (selection_num != 0 || !selectionBelongsToObject(coreObject, selection)) {
-        *start_offset = *end_offset = 0;
-        return 0;
-    }
-
-    *start_offset = selection.start().offsetInContainerNode();
-    *end_offset = selection.end().offsetInContainerNode();
-
-    return webkit_accessible_text_get_text(text, *start_offset, *end_offset);
+    getSelectionOffsetsForObject(coreObject, selection, *startOffset, *endOffset);
+
+    // Return 0 instead of "", as that's the expected result for
+    // this AtkText method when there's no selection
+    if (*startOffset == *endOffset)
+        return 0;
+
+    return webkit_accessible_text_get_text(text, *startOffset, *endOffset);
 }