Changeset 155516 in webkit

Timestamp:

Sep 11, 2013 3:32:00 AM (11 years ago)

Author:

mario@webkit.org

Message:

[GTK] Reimplement atk_text_get_text_*_offset for LINE boundaries
​https://bugs.webkit.org/show_bug.cgi?id=114872

Reviewed by Gustavo Noronha Silva.

Source/WebCore:

Re-implement these functions without using GailTextUtil nor Pango.

• accessibility/atk/WebKitAccessibleInterfaceText.cpp:

(lineAtPositionForAtkBoundary): New helper function to find the
line at a given position considering values of AtkTextBoundary.
(webkitAccessibleTextLineForBoundary): New function,
implementing atk_text_get_text_*_offset for LINE.
(webkitAccessibleTextGetTextForOffset): Replace usage of Gail for
LINE boundaries with webkitAccessibleTextLineForBoundary().

Source/WebKit/gtk:

Fixed wrong unit test.

• tests/testatk.c:

(testWebkitAtkGetTextAtOffsetWithPreformattedText): This test was
reporting a trailing '\n' for some reason for a <pre> block, which
is plainly wrong since, in order to return that, there should be
at least a trailing empty space after that and before the </pre>
closing tag. This is fixed now.
(testWebkitAtkGetTextAtOffsetWithWrappedLines): Uncommented tests
that were previously not passing due to a bug in GailTextUtil.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp (modified) (2 diffs)
• WebKit/gtk/ChangeLog (modified) (1 diff)
• WebKit/gtk/tests/testatk.c (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-09-10  Mario Sanchez Prada  <mario.prada@samsung.com>
+
+        [GTK] Reimplement atk_text_get_text_*_offset for LINE boundaries
+        https://bugs.webkit.org/show_bug.cgi?id=114872
+
+        Reviewed by Gustavo Noronha Silva.
+
+        Re-implement these functions without using GailTextUtil nor Pango.
+
+        * accessibility/atk/WebKitAccessibleInterfaceText.cpp:
+        (lineAtPositionForAtkBoundary): New helper function to find the
+        line at a given position considering values of AtkTextBoundary.
+        (webkitAccessibleTextLineForBoundary): New function,
+        implementing atk_text_get_text_*_offset for LINE.
+        (webkitAccessibleTextGetTextForOffset): Replace usage of Gail for
+        LINE boundaries with webkitAccessibleTextLineForBoundary().
+
 2013-09-11  Andreas Kling  <akling@apple.com>
 

trunk/Source/WebCore/accessibility/atk/WebKitAccessibleInterfaceText.cpp

@@ -974 +974 @@
 }
 
+static VisibleSelection lineAtPositionForAtkBoundary(const AccessibilityObject* coreObject, const VisiblePosition& position, AtkTextBoundary boundaryType)
+{
+    VisiblePosition startPosition;
+    VisiblePosition endPosition;
+
+    switch (boundaryType) {
+    case ATK_TEXT_BOUNDARY_LINE_START:
+        startPosition = isStartOfLine(position) ? position : logicalStartOfLine(position);
+        endPosition = logicalEndOfLine(position);
+
+        // In addition to checking that we are not at the end of a block, we need
+        // to check that endPosition has not UPSTREAM affinity, since that would
+        // cause trouble inside of text controls (we would be advancing too much).
+        if (!isEndOfBlock(endPosition) && endPosition.affinity() != UPSTREAM)
+            endPosition = endPosition.next();
+        break;
+
+    case ATK_TEXT_BOUNDARY_LINE_END:
+        startPosition = isEndOfLine(position) ? position : logicalStartOfLine(position);
+        if (!isStartOfBlock(startPosition))
+            startPosition = startPosition.previous();
+        endPosition = logicalEndOfLine(position);
+        break;
+
+    default:
+        ASSERT_NOT_REACHED();
+    }
+
+    VisibleSelection selectedLine(startPosition, endPosition);
+
+    // We mark the selection as 'upstream' so we can use that information later,
+    // when finding the actual offsets in getSelectionOffsetsForObject().
+    if (boundaryType == ATK_TEXT_BOUNDARY_LINE_END)
+        selectedLine.setAffinity(UPSTREAM);
+
+    return selectedLine;
+}
+
+static char* webkitAccessibleTextLineForBoundary(AtkText* text, int offset, AtkTextBoundary boundaryType, GetTextRelativePosition textPosition, int* startOffset, int* endOffset)
+{
+    AccessibilityObject* coreObject = core(text);
+    Document* document = coreObject->document();
+    if (!document)
+        return emptyTextSelectionAtOffset(0, startOffset, endOffset);
+
+    Node* node = getNodeForAccessibilityObject(coreObject);
+    if (!node)
+        return emptyTextSelectionAtOffset(0, startOffset, endOffset);
+
+    int actualOffset = atkOffsetToWebCoreOffset(text, offset);
+
+    // Besides the usual conversion from ATK offsets to WebCore offsets,
+    // we need to consider the potential embedded objects that might have been
+    // inserted in the text exposed through AtkText when calculating the offset.
+    actualOffset -= numberOfReplacedElementsBeforeOffset(text, actualOffset);
+
+    VisiblePosition caretPosition = coreObject->visiblePositionForIndex(actualOffset);
+    VisibleSelection currentLine = lineAtPositionForAtkBoundary(coreObject, caretPosition, boundaryType);
+
+    // Take into account other relative positions, if needed, by
+    // calculating the new position that we would need to consider.
+    VisiblePosition newPosition = caretPosition;
+    switch (textPosition) {
+    case GetTextPositionAt:
+        // No need to do additional work if we are using the "at" position, we just
+        // explicitly list this case option to catch invalid values in the default case.
+        break;
+
+    case GetTextPositionBefore:
+        // Early return if asking for the previous line while already at the beginning.
+        if (isFirstVisiblePositionInNode(currentLine.visibleStart(), node))
+            return emptyTextSelectionAtOffset(0, startOffset, endOffset);
+        newPosition = currentLine.visibleStart().previous();
+        break;
+
+    case GetTextPositionAfter:
+        // Early return if asking for the following word while already at the end.
+        if (isLastVisiblePositionInNode(currentLine.visibleEnd(), node))
+            return emptyTextSelectionAtOffset(accessibilityObjectLength(coreObject), startOffset, endOffset);
+        newPosition = currentLine.visibleEnd().next();
+        break;
+
+    default:
+        ASSERT_NOT_REACHED();
+    }
+
+    // Determine the relevant line we are actually interested in
+    // and calculate the ATK offsets for it, then return everything.
+    VisibleSelection selectedLine = newPosition != caretPosition ? lineAtPositionForAtkBoundary(coreObject, newPosition, boundaryType) : currentLine;
+    getSelectionOffsetsForObject(coreObject, selectedLine, *startOffset, *endOffset);
+
+    // We might need to adjust the start or end offset to include the list item marker,
+    // if present, when printing the first or the last full line for a list item.
+    RenderObject* renderer = coreObject->renderer();
+    if (renderer->isListItem()) {
+        // For Left-to-Right, the list item marker is at the beginning of the exposed text.
+        if (renderer->style()->direction() == LTR && isFirstVisiblePositionInNode(selectedLine.visibleStart(), node))
+            *startOffset = 0;
+
+        // For Right-to-Left, the list item marker is at the end of the exposed text.
+        if (renderer->style()->direction() == RTL && isLastVisiblePositionInNode(selectedLine.visibleEnd(), node))
+            *endOffset = accessibilityObjectLength(coreObject);
+    }
+
+    return webkitAccessibleTextGetText(text, *startOffset, *endOffset);
+}
+
 static gchar* webkitAccessibleTextGetTextForOffset(AtkText* text, gint offset, AtkTextBoundary boundaryType, GetTextRelativePosition textPosition, gint* startOffset, gint* endOffset)
 {
@@ -988 +1095 @@
     if (boundaryType == ATK_TEXT_BOUNDARY_SENTENCE_START || boundaryType == ATK_TEXT_BOUNDARY_SENTENCE_END)
         return webkitAccessibleTextSentenceForBoundary(text, offset, boundaryType, textPosition, startOffset, endOffset);
+
+    if (boundaryType == ATK_TEXT_BOUNDARY_LINE_START || boundaryType == ATK_TEXT_BOUNDARY_LINE_END)
+        return webkitAccessibleTextLineForBoundary(text, offset, boundaryType, textPosition, startOffset, endOffset);
 
 #if PLATFORM(GTK)

trunk/Source/WebKit/gtk/ChangeLog

@@ +1 @@
+2013-09-10  Mario Sanchez Prada  <mario.prada@samsung.com>
+
+        [GTK] Reimplement atk_text_get_text_*_offset for LINE boundaries
+        https://bugs.webkit.org/show_bug.cgi?id=114872
+
+        Reviewed by Gustavo Noronha Silva.
+
+        Fixed wrong unit test.
+
+        * tests/testatk.c:
+        (testWebkitAtkGetTextAtOffsetWithPreformattedText): This test was
+        reporting a trailing '\n' for some reason for a <pre> block, which
+        is plainly wrong since, in order to return that, there should be
+        at least a trailing empty space after that and before the </pre>
+        closing tag. This is fixed now.
+        (testWebkitAtkGetTextAtOffsetWithWrappedLines): Uncommented tests
+        that were previously not passing due to a bug in GailTextUtil.
+
 2013-09-11  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 

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

@@ -34 +34 @@
 static const char* contentsWithNewlines = "<html><body><p>This is a test. \n\nThis\n is the second sentence. And this the third.</p></body></html>";
 
-static const char* contentsWithPreformattedText = "<html><body><pre>\n\t\n\tfirst line\n\tsecond line\n</pre></body></html>";
+static const char* contentsWithPreformattedText = "<html><body><pre>\n\t\n\tfirst line\n\tsecond line\n\t\n</pre></body></html>";
 
 static const char* contentsWithSpecialChars = "<html><body><p>&laquo;&nbsp;This is a paragraph with &ldquo;special&rdquo; characters inside.&nbsp;&raquo;</p><ul><li style='max-width:100px;'>List item with some text that wraps across different lines.</li><li style='max-width:100px;'><p>List item with some text that wraps across different lines.</p></li></ul></body></html>";
@@ -908 +908 @@
     g_assert(ATK_IS_TEXT(preformattedText));
     char* text = atk_text_get_text(ATK_TEXT(preformattedText), 0, -1);
-    g_assert_cmpstr(text, ==, "\t\n\tfirst line\n\tsecond line\n");
+    g_assert_cmpstr(text, ==, "\t\n\tfirst line\n\tsecond line\n\t\n");
     g_free(text);
 
@@ -1028 +1028 @@
     testGetTextFunction(paragraph1, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_LINE_START, 17, "of the maximum ", 33, 48);
 
-    /* The following line won't work at the moment because of a bug in GailTextUtil.
-       see https://bugzilla.gnome.org/show_bug.cgi?id=703554
-       testGetTextFunction(paragraph1, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_LINE_END, 17, "This is one line", 0, 16); */
+    testGetTextFunction(paragraph1, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_LINE_END, 17, "This is one line", 0, 16);
     testGetTextFunction(paragraph1, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_LINE_END, 17, " wrapped because", 16, 32);
     testGetTextFunction(paragraph1, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_LINE_END, 17, " of the maximum", 32, 47);
@@ -1060 +1058 @@
     testGetTextFunction(paragraph2, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_LINE_START, 30, "line break in the middle.", 51, 76);
 
-    /* The following line won't work at the moment because of a bug in GailTextUtil.
-       see https://bugzilla.gnome.org/show_bug.cgi?id=703554
-       testGetTextFunction(paragraph2, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_LINE_END, 30, "This is another line wrapped", 0, 28); */
+    testGetTextFunction(paragraph2, atk_text_get_text_before_offset, ATK_TEXT_BOUNDARY_LINE_END, 30, "This is another line wrapped", 0, 28);
     testGetTextFunction(paragraph2, atk_text_get_text_at_offset, ATK_TEXT_BOUNDARY_LINE_END, 30, "\nbecause of one forced", 28, 50);
     testGetTextFunction(paragraph2, atk_text_get_text_after_offset, ATK_TEXT_BOUNDARY_LINE_END, 30, "\nline break in the middle.", 50, 76);