Text selection: Fix word boundaries for languages without spaces

WordIterator's getEnd/getBeginning methods does not support the needs
for word boundary detection in text selection. Consider the words
AABB (where AA and BB are each words). If getEnd is given the offset
on the boundary between AA and BB, it would return that offset since
it is the end of AA. For text selection we always want the "next end"
if available.

This CL adds two methods to word iterator that return the next end
and previous beginning.

This CL also alters the code in Start/EndHandle to use the x value
and current / prev line positions to determine if the user is
expanding or contracting the selection.

Bug: 21305292
Change-Id: I6e7a83e53e245d71e43d78f1957f844f2ed1cdfb

2 files changed, 183 insertions, 14 deletions

diff --git a/core/java/android/text/method/WordIterator.java b/core/java/android/text/method/WordIterator.java
index 5dda8a765eb3..3688cfa33d5d 100644
--- a/core/java/android/text/method/WordIterator.java
+++ b/core/java/android/text/method/WordIterator.java
@@ -147,11 +147,90 @@ public class WordIterator implements Selection.PositionIterator {
      * @throws IllegalArgumentException is offset is not valid.
      */
     public int getBeginning(int offset) {
+        // TODO: Check if usage of this can be updated to getBeginning(offset, true) if
+        // so this method can be removed.
+        return getBeginning(offset, false);
+    }
+
+    /**
+     * If <code>offset</code> is within a word, returns the index of the last character of that
+     * word plus one, otherwise returns BreakIterator.DONE.
+     *
+     * The offsets that are considered to be part of a word are the indexes of its characters,
+     * <i>as well as</i> the index of its last character plus one.
+     * If offset is the index of a low surrogate character, BreakIterator.DONE will be returned.
+     *
+     * Valid range for offset is [0..textLength] (note the inclusive upper bound).
+     * The returned value is within [offset..textLength] or BreakIterator.DONE.
+     *
+     * @throws IllegalArgumentException is offset is not valid.
+     */
+    public int getEnd(int offset) {
+        // TODO: Check if usage of this can be updated to getEnd(offset, true), if
+        // so this method can be removed.
+        return getEnd(offset, false);
+    }
+
+    /**
+     * If the <code>offset</code> is within a word or on a word boundary that can only be
+     * considered the start of a word (e.g. _word where "_" is any character that would not
+     * be considered part of the word) then this returns the index of the first character of
+     * that word.
+     *
+     * If the offset is on a word boundary that can be considered the start and end of a
+     * word, e.g. AABB (where AA and BB are both words) and the offset is the boundary
+     * between AA and BB, this would return the start of the previous word, AA.
+     *
+     * Returns BreakIterator.DONE if there is no previous boundary.
+     *
+     * @throws IllegalArgumentException is offset is not valid.
+     */
+    public int getPrevWordBeginningOnTwoWordsBoundary(int offset) {
+        return getBeginning(offset, true);
+    }
+
+    /**
+     * If the <code>offset</code> is within a word or on a word boundary that can only be
+     * considered the end of a word (e.g. word_ where "_" is any character that would not
+     * be considered part of the word) then this returns the index of the last character
+     * plus one of that word.
+     *
+     * If the offset is on a word boundary that can be considered the start and end of a
+     * word, e.g. AABB (where AA and BB are both words) and the offset is the boundary
+     * between AA and BB, this would return the end of the next word, BB.
+     *
+     * Returns BreakIterator.DONE if there is no next boundary.
+     *
+     * @throws IllegalArgumentException is offset is not valid.
+     */
+    public int getNextWordEndOnTwoWordBoundary(int offset) {
+        return getEnd(offset, true);
+    }
+
+    /**
+     * If the <code>offset</code> is within a word or on a word boundary that can only be
+     * considered the start of a word (e.g. _word where "_" is any character that would not
+     * be considered part of the word) then this returns the index of the first character of
+     * that word.
+     *
+     * If the offset is on a word boundary that can be considered the start and end of a
+     * word, e.g. AABB (where AA and BB are both words) and the offset is the boundary
+     * between AA and BB, and getPrevWordBeginningOnTwoWordsBoundary is true then this would
+     * return the start of the previous word, AA. Otherwise it would return the current offset,
+     * the start of BB.
+     *
+     * Returns BreakIterator.DONE if there is no previous boundary.
+     *
+     * @throws IllegalArgumentException is offset is not valid.
+     */
+    private int getBeginning(int offset, boolean getPrevWordBeginningOnTwoWordsBoundary) {
         final int shiftedOffset = offset - mOffsetShift;
         checkOffsetIsValid(shiftedOffset);
 
         if (isOnLetterOrDigit(shiftedOffset)) {
-            if (mIterator.isBoundary(shiftedOffset)) {
+            if (mIterator.isBoundary(shiftedOffset)
+                    && (!isAfterLetterOrDigit(shiftedOffset)
+                            || !getPrevWordBeginningOnTwoWordsBoundary)) {
                 return shiftedOffset + mOffsetShift;
             } else {
                 return mIterator.preceding(shiftedOffset) + mOffsetShift;
@@ -164,24 +243,29 @@ public class WordIterator implements Selection.PositionIterator {
         return BreakIterator.DONE;
     }
 
-    /** If <code>offset</code> is within a word, returns the index of the last character of that
-     * word plus one, otherwise returns BreakIterator.DONE.
+    /**
+     * If the <code>offset</code> is within a word or on a word boundary that can only be
+     * considered the end of a word (e.g. word_ where "_" is any character that would not be
+     * considered part of the word) then this returns the index of the last character plus one
+     * of that word.
      *
-     * The offsets that are considered to be part of a word are the indexes of its characters,
-     * <i>as well as</i> the index of its last character plus one.
-     * If offset is the index of a low surrogate character, BreakIterator.DONE will be returned.
+     * If the offset is on a word boundary that can be considered the start and end of a
+     * word, e.g. AABB (where AA and BB are both words) and the offset is the boundary
+     * between AA and BB, and getNextWordEndOnTwoWordBoundary is true then this would return
+     * the end of the next word, BB. Otherwise it would return the current offset, the end
+     * of AA.
      *
-     * Valid range for offset is [0..textLength] (note the inclusive upper bound).
-     * The returned value is within [offset..textLength] or BreakIterator.DONE.
+     * Returns BreakIterator.DONE if there is no next boundary.
      *
      * @throws IllegalArgumentException is offset is not valid.
      */
-    public int getEnd(int offset) {
+    private int getEnd(int offset, boolean getNextWordEndOnTwoWordBoundary) {
         final int shiftedOffset = offset - mOffsetShift;
         checkOffsetIsValid(shiftedOffset);
 
         if (isAfterLetterOrDigit(shiftedOffset)) {
-            if (mIterator.isBoundary(shiftedOffset)) {
+            if (mIterator.isBoundary(shiftedOffset)
+                    && (!isOnLetterOrDigit(shiftedOffset) || !getNextWordEndOnTwoWordBoundary)) {
                 return shiftedOffset + mOffsetShift;
             } else {
                 return mIterator.following(shiftedOffset) + mOffsetShift;
diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java
index e050bdab63f6..fa7c8e649e70 100644
--- a/core/java/android/widget/Editor.java
+++ b/core/java/android/widget/Editor.java
@@ -123,6 +123,7 @@ public class Editor {
     private static final float[] TEMP_POSITION = new float[2];
     private static int DRAG_SHADOW_MAX_TEXT_LENGTH = 20;
     private static final float LINE_SLOP_MULTIPLIER_FOR_HANDLEVIEWS = 0.5f;
+    private static final int UNSET_X_VALUE = -1;
     // Tag used when the Editor maintains its own separate UndoManager.
     private static final String UNDO_OWNER_TAG = "Editor";
 
@@ -735,7 +736,7 @@ public class Editor {
             retOffset = getWordIteratorWithText().getPunctuationBeginning(offset);
         } else {
             // Not on a punctuation boundary, find the word start.
-            retOffset = getWordIteratorWithText().getBeginning(offset);
+            retOffset = getWordIteratorWithText().getPrevWordBeginningOnTwoWordsBoundary(offset);
         }
         if (retOffset == BreakIterator.DONE) {
             return offset;
@@ -750,7 +751,7 @@ public class Editor {
             retOffset = getWordIteratorWithText().getPunctuationEnd(offset);
         } else {
             // Not on a punctuation boundary, find the word end.
-            retOffset = getWordIteratorWithText().getEnd(offset);
+            retOffset = getWordIteratorWithText().getNextWordEndOnTwoWordBoundary(offset);
         }
         if (retOffset == BreakIterator.DONE) {
             return offset;
@@ -4058,6 +4059,10 @@ public class Editor {
         private boolean mInWord = false;
         // Difference between touch position and word boundary position.
         private float mTouchWordDelta;
+        // X value of the previous updatePosition call.
+        private float mPrevX;
+        // Indicates if the handle has moved a boundary between LTR and RTL text.
+        private boolean mLanguageDirectionChanged = false;
 
         public SelectionStartHandleView(Drawable drawableLtr, Drawable drawableRtl) {
             super(drawableLtr, drawableRtl);
@@ -4118,7 +4123,43 @@ public class Editor {
             int end = getWordEnd(offset);
             int start = getWordStart(offset);
 
-            if (offset < mPreviousOffset) {
+            if (mPrevX == UNSET_X_VALUE) {
+                mPrevX = x;
+            }
+
+            final int selectionStart = mTextView.getSelectionStart();
+            final boolean selectionStartRtl = layout.isRtlCharAt(selectionStart);
+            final boolean atRtl = layout.isRtlCharAt(offset);
+            final boolean isLvlBoundary = layout.isLevelBoundary(offset);
+            boolean isExpanding;
+
+            // We can't determine if the user is expanding or shrinking the selection if they're
+            // on a bi-di boundary, so until they've moved past the boundary we'll just place
+            // the cursor at the current position.
+            if (isLvlBoundary || (selectionStartRtl && !atRtl) || (!selectionStartRtl && atRtl)) {
+                // We're on a boundary or this is the first direction change -- just update
+                // to the current position.
+                mLanguageDirectionChanged = true;
+                mTouchWordDelta = 0.0f;
+                positionAtCursorOffset(offset, false);
+                return;
+            } else if (mLanguageDirectionChanged && !isLvlBoundary) {
+                // We've just moved past the boundary so update the position. After this we can
+                // figure out if the user is expanding or shrinking to go by word or character.
+                positionAtCursorOffset(offset, false);
+                mTouchWordDelta = 0.0f;
+                mLanguageDirectionChanged = false;
+                return;
+            } else {
+                final float xDiff = x - mPrevX;
+                if (atRtl) {
+                    isExpanding = xDiff > 0 || currLine > mPrevLine;
+                } else {
+                    isExpanding = xDiff < 0 || currLine < mPrevLine;
+                }
+            }
+
+            if (isExpanding) {
                 // User is increasing the selection.
                 if (!mInWord || currLine < mPrevLine) {
                     // We're not in a word, or we're on a different line so we'll expand by
@@ -4173,6 +4214,7 @@ public class Editor {
                 }
                 positionAtCursorOffset(offset, false);
             }
+            mPrevX = x;
         }
 
         @Override
@@ -4187,6 +4229,7 @@ public class Editor {
             if (event.getActionMasked() == MotionEvent.ACTION_UP) {
                 // Reset the touch word offset when the user has lifted their finger.
                 mTouchWordDelta = 0.0f;
+                mPrevX = UNSET_X_VALUE;
             }
             return superResult;
         }
@@ -4197,6 +4240,10 @@ public class Editor {
         private boolean mInWord = false;
         // Difference between touch position and word boundary position.
         private float mTouchWordDelta;
+        // X value of the previous updatePosition call.
+        private float mPrevX;
+        // Indicates if the handle has moved a boundary between LTR and RTL text.
+        private boolean mLanguageDirectionChanged = false;
 
         public SelectionEndHandleView(Drawable drawableLtr, Drawable drawableRtl) {
             super(drawableLtr, drawableRtl);
@@ -4257,7 +4304,43 @@ public class Editor {
             int end = getWordEnd(offset);
             int start = getWordStart(offset);
 
-            if (offset > mPreviousOffset) {
+            if (mPrevX == UNSET_X_VALUE) {
+                mPrevX = x;
+            }
+
+            final int selectionEnd = mTextView.getSelectionEnd();
+            final boolean selectionEndRtl = layout.isRtlCharAt(selectionEnd);
+            final boolean atRtl = layout.isRtlCharAt(offset);
+            final boolean isLvlBoundary = layout.isLevelBoundary(offset);
+            boolean isExpanding;
+
+            // We can't determine if the user is expanding or shrinking the selection if they're
+            // on a bi-di boundary, so until they've moved past the boundary we'll just place
+            // the cursor at the current position.
+            if (isLvlBoundary || (selectionEndRtl && !atRtl) || (!selectionEndRtl && atRtl)) {
+                // We're on a boundary or this is the first direction change -- just update
+                // to the current position.
+                mLanguageDirectionChanged = true;
+                mTouchWordDelta = 0.0f;
+                positionAtCursorOffset(offset, false);
+                return;
+            } else if (mLanguageDirectionChanged && !isLvlBoundary) {
+                // We've just moved past the boundary so update the position. After this we can
+                // figure out if the user is expanding or shrinking to go by word or character.
+                positionAtCursorOffset(offset, false);
+                mTouchWordDelta = 0.0f;
+                mLanguageDirectionChanged = false;
+                return;
+            } else {
+                final float xDiff = x - mPrevX;
+                if (atRtl) {
+                    isExpanding = xDiff < 0 || currLine < mPrevLine;
+                } else {
+                    isExpanding = xDiff > 0 || currLine > mPrevLine;
+                }
+            }
+
+            if (isExpanding) {
                 // User is increasing the selection.
                 if (!mInWord || currLine > mPrevLine) {
                     // We're not in a word, or we're on a different line so we'll expand by
@@ -4312,6 +4395,7 @@ public class Editor {
                 }
                 positionAtCursorOffset(offset, false);
             }
+            mPrevX = x;
         }
 
         @Override
@@ -4326,6 +4410,7 @@ public class Editor {
             if (event.getActionMasked() == MotionEvent.ACTION_UP) {
                 // Reset the touch word offset when the user has lifted their finger.
                 mTouchWordDelta = 0.0f;
+                mPrevX = UNSET_X_VALUE;
             }
             return superResult;
         }