Fix issue where handle is in front of word rather than end of word

Previous getWordEnd and getWordStart functions would return a boundary
rather than a word end / start boundary in some cases. This behavior is
most evident when moving between short lines -- the handle would go to
the next boundary rather than the word end on the next line (and the
start handle would go to the word end rather than the word start on
the prev line).

This CL ensures that word or punctuation boundaries are returned and moves
most of the punctuation boundary logic into WordIterator since it makes
a bit more sense there.

Bug: 21030788
Change-Id: I96c6aff7f2c213aa3c4f66ac87ca913ca16fd347

2 files changed, 93 insertions, 80 deletions

diff --git a/core/java/android/text/method/WordIterator.java b/core/java/android/text/method/WordIterator.java
index c4dc5ed..5dda8a7 100644
--- a/core/java/android/text/method/WordIterator.java
+++ b/core/java/android/text/method/WordIterator.java
@@ -194,6 +194,87 @@ public class WordIterator implements Selection.PositionIterator {
         return BreakIterator.DONE;
     }
 
+    /**
+     * If <code>offset</code> is within a group of punctuation as defined
+     * by {@link #isPunctuation(int)}, returns the index of the first character
+     * of that group, otherwise returns BreakIterator.DONE.
+     *
+     * @param offset the offset to search from.
+     */
+    public int getPunctuationBeginning(int offset) {
+        while (offset != BreakIterator.DONE && !isPunctuationStartBoundary(offset)) {
+            offset = prevBoundary(offset);
+        }
+        // No need to shift offset, prevBoundary handles that.
+        return offset;
+    }
+
+    /**
+     * If <code>offset</code> is within a group of punctuation as defined
+     * by {@link #isPunctuation(int)}, returns the index of the last character
+     * of that group plus one, otherwise returns BreakIterator.DONE.
+     *
+     * @param offset the offset to search from.
+     */
+    public int getPunctuationEnd(int offset) {
+        while (offset != BreakIterator.DONE && !isPunctuationEndBoundary(offset)) {
+            offset = nextBoundary(offset);
+        }
+        // No need to shift offset, nextBoundary handles that.
+        return offset;
+    }
+
+    /**
+     * Indicates if the provided offset is after a punctuation character
+     * as defined by {@link #isPunctuation(int)}.
+     *
+     * @param offset the offset to check from.
+     * @return Whether the offset is after a punctuation character.
+     */
+    public boolean isAfterPunctuation(int offset) {
+        final int shiftedOffset = offset - mOffsetShift;
+        if (shiftedOffset >= 1 && shiftedOffset <= mString.length()) {
+            final int codePoint = mString.codePointBefore(shiftedOffset);
+            return isPunctuation(codePoint);
+        }
+        return false;
+    }
+
+    /**
+     * Indicates if the provided offset is at a punctuation character
+     * as defined by {@link #isPunctuation(int)}.
+     *
+     * @param offset the offset to check from.
+     * @return Whether the offset is at a punctuation character.
+     */
+    public boolean isOnPunctuation(int offset) {
+        final int shiftedOffset = offset - mOffsetShift;
+        if (shiftedOffset >= 0 && shiftedOffset < mString.length()) {
+            final int codePoint = mString.codePointAt(shiftedOffset);
+            return isPunctuation(codePoint);
+        }
+        return false;
+    }
+
+    private boolean isPunctuationStartBoundary(int offset) {
+        return isOnPunctuation(offset) && !isAfterPunctuation(offset);
+    }
+
+    private boolean isPunctuationEndBoundary(int offset) {
+        return !isOnPunctuation(offset) && isAfterPunctuation(offset);
+    }
+
+    private boolean isPunctuation(int cp) {
+        int type = Character.getType(cp);
+        return (type == Character.CONNECTOR_PUNCTUATION ||
+                type == Character.DASH_PUNCTUATION ||
+                type == Character.END_PUNCTUATION ||
+                type == Character.FINAL_QUOTE_PUNCTUATION ||
+                type == Character.INITIAL_QUOTE_PUNCTUATION ||
+                type == Character.OTHER_PUNCTUATION ||
+                type == Character.START_PUNCTUATION);
+    }
+
     private boolean isAfterLetterOrDigit(int shiftedOffset) {
         if (shiftedOffset >= 1 && shiftedOffset <= mString.length()) {
             final int codePoint = mString.codePointBefore(shiftedOffset);
diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java
index 4fd85b6..95c5062 100644
--- a/core/java/android/widget/Editor.java
+++ b/core/java/android/widget/Editor.java
@@ -689,14 +689,12 @@ public class Editor {
         // FIXME - For this and similar methods we're not doing anything to check if there's
         // a LocaleSpan in the text, this may be something we should try handling or checking for.
         int retOffset = getWordIteratorWithText().prevBoundary(offset);
-        if (isPunctBoundaryBehind(retOffset, true /* isStart */)) {
-            // If we're on a punctuation boundary we should continue to get the
-            // previous offset until we're not longer on a punctuation boundary.
-            retOffset = getWordIteratorWithText().prevBoundary(retOffset);
-            while (!isPunctBoundaryBehind(retOffset, false /* isStart */)
-                    && retOffset != BreakIterator.DONE) {
-                retOffset = getWordIteratorWithText().prevBoundary(retOffset);
-            }
+        if (getWordIteratorWithText().isOnPunctuation(retOffset)) {
+            // On punctuation boundary or within group of punctuation, find punctuation start.
+            retOffset = getWordIteratorWithText().getPunctuationBeginning(offset);
+        } else {
+            // Not on a punctuation boundary, find the word start.
+            retOffset = getWordIteratorWithText().getBeginning(offset);
         }
         if (retOffset == BreakIterator.DONE) {
             return offset;
@@ -706,14 +704,12 @@ public class Editor {
 
     private int getWordEnd(int offset) {
         int retOffset = getWordIteratorWithText().nextBoundary(offset);
-        if (isPunctBoundaryForward(retOffset, true /* isStart */)) {
-            // If we're on a punctuation boundary we should continue to get the
-            // next offset until we're no longer on a punctuation boundary.
-            retOffset = getWordIteratorWithText().nextBoundary(retOffset);
-            while (!isPunctBoundaryForward(retOffset, false /* isStart */)
-                    && retOffset != BreakIterator.DONE) {
-                retOffset = getWordIteratorWithText().nextBoundary(retOffset);
-            }
+        if (getWordIteratorWithText().isAfterPunctuation(retOffset)) {
+            // On punctuation boundary or within group of punctuation, find punctuation end.
+            retOffset = getWordIteratorWithText().getPunctuationEnd(offset);
+        } else {
+            // Not on a punctuation boundary, find the word end.
+            retOffset = getWordIteratorWithText().getEnd(offset);
         }
         if (retOffset == BreakIterator.DONE) {
             return offset;
@@ -722,70 +718,6 @@ public class Editor {
     }
 
     /**
-     * Checks for punctuation boundaries for the provided offset and the
-     * previous character.
-     *
-     * @param offset The offset to check from.
-     * @param isStart Whether the boundary being checked for is at the start or
-     *            end of a punctuation sequence.
-     * @return Whether this is a punctuation boundary.
-     */
-    private boolean isPunctBoundaryBehind(int offset, boolean isStart) {
-        CharSequence text = mTextView.getText();
-        if (offset == BreakIterator.DONE || offset > text.length() || offset == 0) {
-            return false;
-        }
-        int cp = Character.codePointAt(text, offset);
-        int prevCp = Character.codePointBefore(text, offset);
-
-        if (isPunctuation(cp)) {
-            // If it's the start, the current cp and the prev cp are
-            // punctuation. If it's at the end of a punctuation sequence the
-            // current is punctuation and the prev is not.
-            return isStart ? isPunctuation(prevCp) : !isPunctuation(prevCp);
-        }
-        return false;
-    }
-
-    /**
-     * Checks for punctuation boundaries for the provided offset and the next
-     * character.
-     *
-     * @param offset The offset to check from.
-     * @param isStart Whether the boundary being checked for is at the start or
-     *            end of a punctuation sequence.
-     * @return Whether this is a punctuation boundary.
-     */
-    private boolean isPunctBoundaryForward(int offset, boolean isStart) {
-        CharSequence text = mTextView.getText();
-        if (offset == BreakIterator.DONE || offset > text.length() || offset == 0) {
-            return false;
-        }
-        int cp = Character.codePointBefore(text, offset);
-        int nextCpOffset = Math.min(offset + Character.charCount(cp), text.length() - 1);
-        int nextCp = Character.codePointBefore(text, nextCpOffset);
-
-        if (isPunctuation(cp)) {
-            // If it's the start, the current cp and the next cp are
-            // punctuation. If it's at the end of a punctuation sequence the
-            // current is punctuation and the next is not.
-            return isStart ? isPunctuation(nextCp) : !isPunctuation(nextCp);
-        }
-        return false;
-    }
-
-    private boolean isPunctuation(int cp) {
-        int type = Character.getType(cp);
-        return (type == Character.CONNECTOR_PUNCTUATION ||
-                type == Character.DASH_PUNCTUATION ||
-                type == Character.END_PUNCTUATION ||
-                type == Character.FINAL_QUOTE_PUNCTUATION ||
-                type == Character.INITIAL_QUOTE_PUNCTUATION ||
-                type == Character.OTHER_PUNCTUATION ||
-                type == Character.START_PUNCTUATION);
-    }
-
-    /**
      * Adjusts selection to the word under last touch offset. Return true if the operation was
      * successfully performed.
      */