From 58c9087137989da8411ffd212072f630d3fac4f3 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Tue, 12 May 2015 11:09:37 -0700 Subject: [PATCH] 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 --- .../android/text/method/WordIterator.java | 81 ++++++++++++++++ core/java/android/widget/Editor.java | 92 +++---------------- 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 c4dc5edc1592a..5dda8a765eb37 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 offset 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 offset 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 4fd85b6ea953b..95c506263dcd2 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; @@ -721,70 +717,6 @@ public class Editor { return retOffset; } - /** - * 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.