From 7c263ddd582abacbbe9c71341a6ef4c704d12100 Mon Sep 17 00:00:00 2001 From: Keisuke Kuroyanagi Date: Thu, 12 Jan 2017 19:24:18 +0900 Subject: [PATCH] Make selection end handle stick to selection at line end. At line break, one offset can be mapped to two phisical position: previous line end and next line start. Previously, all cursor handles are placed at next line start. With this CL, selection end handle is placed at the previous line end in such cases. Test: FrameworksCoreTests Bug: 21305922 Change-Id: I00d9f9a0cd417ca92534e93b3d3f655cd62f25d3 --- core/java/android/text/Layout.java | 89 ++++++++++++------ core/java/android/widget/Editor.java | 51 ++++++++--- core/java/android/widget/TextView.java | 2 +- .../android/widget/TextViewActivityTest.java | 23 +++++ .../widget/espresso/TextViewActions.java | 90 +++++++++++++------ .../widget/espresso/TextViewAssertions.java | 38 ++++++++ 6 files changed, 228 insertions(+), 65 deletions(-) diff --git a/core/java/android/text/Layout.java b/core/java/android/text/Layout.java index fd6fc7dc0860f..fb35fed4d9ef7 100644 --- a/core/java/android/text/Layout.java +++ b/core/java/android/text/Layout.java @@ -908,17 +908,24 @@ public abstract class Layout { * the paragraph's primary direction. */ public float getPrimaryHorizontal(int offset) { - return getPrimaryHorizontal(offset, false /* not clamped */); + return getPrimaryHorizontal(offset, false /* not clamped */, + true /* getNewLineStartPosOnLineBreak */); } /** * Get the primary horizontal position for the specified text offset, but * optionally clamp it so that it doesn't exceed the width of the layout. + * + * @param offset the offset to get horizontal position + * @param clamped whether to clamp the position by using the width of this layout. + * @param getNewLineStartPosOnLineBreak whether to get the start position of new line when the + * offset is at automatic line break. * @hide */ - public float getPrimaryHorizontal(int offset, boolean clamped) { + public float getPrimaryHorizontal(int offset, boolean clamped, + boolean getNewLineStartPosOnLineBreak) { boolean trailing = primaryIsTrailingPrevious(offset); - return getHorizontal(offset, trailing, clamped); + return getHorizontal(offset, trailing, clamped, getNewLineStartPosOnLineBreak); } /** @@ -927,26 +934,37 @@ public abstract class Layout { * the direction other than the paragraph's primary direction. */ public float getSecondaryHorizontal(int offset) { - return getSecondaryHorizontal(offset, false /* not clamped */); + return getSecondaryHorizontal(offset, false /* not clamped */, + true /* getNewLineStartPosOnLineBreak */); } /** * Get the secondary horizontal position for the specified text offset, but * optionally clamp it so that it doesn't exceed the width of the layout. + * + * @param offset the offset to get horizontal position + * @param clamped whether to clamp the position by using the width of this layout. + * @param getNewLineStartPosOnLineBreak whether to get the start position of new line when the + * offset is at automatic line break. * @hide */ - public float getSecondaryHorizontal(int offset, boolean clamped) { + public float getSecondaryHorizontal(int offset, boolean clamped, + boolean getNewLineStartPosOnLineBreak) { boolean trailing = primaryIsTrailingPrevious(offset); - return getHorizontal(offset, !trailing, clamped); + return getHorizontal(offset, !trailing, clamped, getNewLineStartPosOnLineBreak); } - private float getHorizontal(int offset, boolean primary) { - return primary ? getPrimaryHorizontal(offset) : getSecondaryHorizontal(offset); + private float getHorizontal(int offset, boolean primary, + boolean getNewLineStartPosOnLineBreak) { + return primary ? getPrimaryHorizontal(offset, false /* not clamped */, + getNewLineStartPosOnLineBreak) + : getSecondaryHorizontal(offset, false /* not clamped */, + getNewLineStartPosOnLineBreak); } - private float getHorizontal(int offset, boolean trailing, boolean clamped) { - int line = getLineForOffset(offset); - + private float getHorizontal(int offset, boolean trailing, boolean clamped, + boolean getNewLineStartPosOnLineBreak) { + final int line = getLineForOffset(offset, getNewLineStartPosOnLineBreak); return getHorizontal(offset, trailing, line, clamped); } @@ -1150,6 +1168,10 @@ public abstract class Layout { * beyond the end of the text, you get the last line. */ public int getLineForOffset(int offset) { + return getLineForOffset(offset, true); + } + + private int getLineForOffset(int offset, boolean getNewLineOnLineBreak) { int high = getLineCount(), low = -1, guess; while (high - low > 1) { @@ -1161,10 +1183,15 @@ public abstract class Layout { low = guess; } - if (low < 0) + if (low < 0) { return 0; - else + } else { + if (!getNewLineOnLineBreak && low > 0 && getLineStart(low) == offset + && mText.charAt(offset - 1) != '\n') { + return low - 1; + } return low; + } } /** @@ -1198,14 +1225,14 @@ public abstract class Layout { false, null); final int max; - if (line == getLineCount() - 1) { - max = lineEndOffset; - } else { + if (line != getLineCount() - 1 && mText.charAt(lineEndOffset - 1) == '\n') { max = tl.getOffsetToLeftRightOf(lineEndOffset - lineStartOffset, !isRtlCharAt(lineEndOffset - 1)) + lineStartOffset; + } else { + max = lineEndOffset; } int best = lineStartOffset; - float bestdist = Math.abs(getHorizontal(best, primary) - horiz); + float bestdist = Math.abs(getHorizontal(best, primary, true) - horiz); for (int i = 0; i < dirs.mDirections.length; i += 2) { int here = lineStartOffset + dirs.mDirections[i]; @@ -1221,10 +1248,13 @@ public abstract class Layout { guess = (high + low) / 2; int adguess = getOffsetAtStartOf(guess); - if (getHorizontal(adguess, primary) * swap >= horiz * swap) + if (getHorizontal(adguess, primary, + adguess == lineStartOffset || adguess != lineEndOffset) * swap + >= horiz * swap) { high = guess; - else + } else { low = guess; + } } if (low < here + 1) @@ -1234,9 +1264,11 @@ public abstract class Layout { int aft = tl.getOffsetToLeftRightOf(low - lineStartOffset, isRtl) + lineStartOffset; low = tl.getOffsetToLeftRightOf(aft - lineStartOffset, !isRtl) + lineStartOffset; if (low >= here && low < there) { - float dist = Math.abs(getHorizontal(low, primary) - horiz); + float dist = Math.abs(getHorizontal(low, primary, + low == lineStartOffset || low != lineEndOffset) - horiz); if (aft < there) { - float other = Math.abs(getHorizontal(aft, primary) - horiz); + float other = Math.abs(getHorizontal(aft, primary, + aft == lineStartOffset || aft != lineEndOffset) - horiz); if (other < dist) { dist = other; @@ -1251,7 +1283,8 @@ public abstract class Layout { } } - float dist = Math.abs(getHorizontal(here, primary) - horiz); + float dist = Math.abs(getHorizontal(here, primary, + here == lineStartOffset || here != lineEndOffset) - horiz); if (dist < bestdist) { bestdist = dist; @@ -1259,10 +1292,10 @@ public abstract class Layout { } } - float dist = Math.abs(getHorizontal(max, primary) - horiz); + float dist = Math.abs(getHorizontal(max, primary, + max == lineStartOffset || max != lineEndOffset) - horiz); if (dist <= bestdist) { - bestdist = dist; best = max; } @@ -1459,8 +1492,9 @@ public abstract class Layout { int bottom = getLineTop(line+1); boolean clamped = shouldClampCursor(line); - float h1 = getPrimaryHorizontal(point, clamped) - 0.5f; - float h2 = isLevelBoundary(point) ? getSecondaryHorizontal(point, clamped) - 0.5f : h1; + float h1 = getPrimaryHorizontal(point, clamped, true) - 0.5f; + float h2 = isLevelBoundary(point) + ? getSecondaryHorizontal(point, clamped, true) - 0.5f : h1; int caps = TextKeyListener.getMetaState(editingBuffer, TextKeyListener.META_SHIFT_ON) | TextKeyListener.getMetaState(editingBuffer, TextKeyListener.META_SELECTING); @@ -1577,8 +1611,7 @@ public abstract class Layout { } int startline = getLineForOffset(start); - int endline = getLineForOffset(end); - + int endline = getLineForOffset(end, false); int top = getLineTop(startline); int bottom = getLineBottom(endline); diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java index 5eaabe7c137b3..c04347c8aca21 100644 --- a/core/java/android/widget/Editor.java +++ b/core/java/android/widget/Editor.java @@ -1931,10 +1931,11 @@ public class Editor { } boolean clamped = layout.shouldClampCursor(line); - updateCursorPosition(0, top, middle, layout.getPrimaryHorizontal(offset, clamped)); + updateCursorPosition(0, top, middle, layout.getPrimaryHorizontal(offset, clamped, true)); if (mCursorCount == 2) { - updateCursorPosition(1, middle, bottom, layout.getSecondaryHorizontal(offset, clamped)); + updateCursorPosition(1, middle, bottom, + layout.getSecondaryHorizontal(offset, clamped, true)); } } @@ -4331,7 +4332,7 @@ public class Editor { updateSelection(offset); addPositionToTouchUpFilter(offset); } - final int line = layout.getLineForOffset(offset); + final int line = getLineForOffset(layout, offset); mPrevLine = line; mPositionX = getCursorHorizontalPosition(layout, offset) - mHotspotX @@ -4358,6 +4359,15 @@ public class Editor { return (int) (getHorizontal(layout, offset) - 0.5f); } + /** + * @param layout Text layout. + * @param offset Character offset for the cursor. + * @return The line the cursor should be at. + */ + int getLineForOffset(Layout layout, int offset) { + return layout.getLineForOffset(offset); + } + @Override public void updatePosition(int parentPositionX, int parentPositionY, boolean parentPositionChanged, boolean parentScrolled) { @@ -4786,7 +4796,7 @@ public class Editor { || !isStartHandle() && initialOffset <= anotherHandleOffset) { // Handles have crossed, bound it to the first selected line and // adjust by word / char as normal. - currLine = layout.getLineForOffset(anotherHandleOffset); + currLine = getLineForOffset(layout, anotherHandleOffset, !isStartHandle()); initialOffset = getOffsetAtCoordinate(layout, currLine, x); } @@ -4858,14 +4868,18 @@ public class Editor { if (isExpanding) { // User is increasing the selection. int wordBoundary = isStartHandle() ? wordStart : wordEnd; - final boolean snapToWord = (!mInWord - || (isStartHandle() ? currLine < mPrevLine : currLine > mPrevLine)) - && atRtl == isAtRtlRun(layout, wordBoundary); + final boolean atLineBoundary = layout.getLineStart(currLine) == offset + || layout.getLineEnd(currLine) == offset; + final boolean atWordBoundary = getWordIteratorWithText().isBoundary(offset); + final boolean snapToWord = !(atLineBoundary && atWordBoundary) + && (!mInWord + || (isStartHandle() ? currLine < mPrevLine : currLine > mPrevLine)) + && atRtl == isAtRtlRun(layout, wordBoundary); if (snapToWord) { // Sometimes words can be broken across lines (Chinese, hyphenation). // We still snap to the word boundary but we only use the letters on the // current line to determine if the user is far enough into the word to snap. - if (layout.getLineForOffset(wordBoundary) != currLine) { + if (getLineForOffset(layout, wordBoundary) != currLine) { wordBoundary = isStartHandle() ? layout.getLineStart(currLine) : layout.getLineEnd(currLine); } @@ -5013,12 +5027,29 @@ public class Editor { } private float getHorizontal(@NonNull Layout layout, int offset, boolean startHandle) { - final int line = layout.getLineForOffset(offset); + final int line = getLineForOffset(layout, offset); final int offsetToCheck = startHandle ? offset : Math.max(offset - 1, 0); final boolean isRtlChar = layout.isRtlCharAt(offsetToCheck); final boolean isRtlParagraph = layout.getParagraphDirection(line) == -1; return (isRtlChar == isRtlParagraph) - ? layout.getPrimaryHorizontal(offset) : layout.getSecondaryHorizontal(offset); + ? layout.getPrimaryHorizontal(offset, false, startHandle) + : layout.getSecondaryHorizontal(offset, false, startHandle); + } + + @Override + public int getLineForOffset(@NonNull Layout layout, int offset) { + return getLineForOffset(layout, offset, isStartHandle()); + } + + private int getLineForOffset(@NonNull Layout layout, int offset, boolean startHandle) { + final int line = layout.getLineForOffset(offset); + if (!startHandle && line > 0 && layout.getLineStart(line) == offset + && mTextView.getText().charAt(offset - 1) != '\n') { + // If end handle is at a line break in a paragraph, the handle should be at the + // previous line. + return line - 1; + } + return line; } @Override diff --git a/core/java/android/widget/TextView.java b/core/java/android/widget/TextView.java index 5426a37cdd804..e9089118f64a3 100644 --- a/core/java/android/widget/TextView.java +++ b/core/java/android/widget/TextView.java @@ -7719,7 +7719,7 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener // right where it is most likely to be annoying. final boolean clamped = grav > 0; // FIXME: Is it okay to truncate this, or should we round? - final int x = (int) layout.getPrimaryHorizontal(offset, clamped); + final int x = (int) layout.getPrimaryHorizontal(offset, clamped, true); final int top = layout.getLineTop(line); final int bottom = layout.getLineTop(line + 1); diff --git a/core/tests/coretests/src/android/widget/TextViewActivityTest.java b/core/tests/coretests/src/android/widget/TextViewActivityTest.java index 71dd526458563..b276d16adae8f 100644 --- a/core/tests/coretests/src/android/widget/TextViewActivityTest.java +++ b/core/tests/coretests/src/android/widget/TextViewActivityTest.java @@ -26,6 +26,7 @@ import static android.widget.espresso.TextViewActions.dragHandle; import static android.widget.espresso.TextViewActions.Handle; import static android.widget.espresso.TextViewActions.longPressAndDragOnText; import static android.widget.espresso.TextViewActions.longPressOnTextAtIndex; +import static android.widget.espresso.TextViewAssertions.handleIsOnLine; import static android.widget.espresso.TextViewAssertions.hasInsertionPointerAtIndex; import static android.widget.espresso.TextViewAssertions.hasSelection; import static android.widget.espresso.FloatingToolbarEspressoUtils.assertFloatingToolbarIsDisplayed; @@ -464,6 +465,28 @@ public class TextViewActivityTest extends ActivityInstrumentationTestCase2 + *
+ * View constraints: + *