diff options
| author | 2024-07-29 01:52:57 -0700 | |
|---|---|---|
| committer | 2024-08-01 19:19:48 -0700 | |
| commit | f51becbf3aafaa92ba299b84f7eae42258c68073 (patch) | |
| tree | b05b9d372cb4375fe61ef579ef2c05d52da8a854 | |
| parent | 5d157ceeb2c760cca05d90fbf0f1877d0f8eebfe (diff) | |
Make the insert mode highlight sticky
This CL fixes the issue that in some Apps the insert mode highlight
is applied to the entire text range after an editing. This is caused
by App call text.replace(0, text.length(), newText) after each edit.
Here the newText is usually a copy of text that has different spans.
And Apps use this trick to update the spans on the text. However, it
tricks the insert mode to think that the entire string is replaced.
This Cl fixed the issue by making the highlight range sticky when the
text replace range covers the highlight range.
Bug: 355137282
Test: atest InsertModeTransformationMethod
Flag: com.android.text.flags.insert_mode_highlight_range
Change-Id: I46dd0f22db0287a51a6f8fda6ea2c8514c2e4b63
3 files changed, 274 insertions, 10 deletions
diff --git a/core/java/android/text/flags/flags.aconfig b/core/java/android/text/flags/flags.aconfig index 88a1b9c562d3..1b85191015a1 100644 --- a/core/java/android/text/flags/flags.aconfig +++ b/core/java/android/text/flags/flags.aconfig @@ -118,6 +118,13 @@ flag { } flag { + name: "insert_mode_highlight_range" + namespace: "text" + description: "Make the highlight range stick after editing, this handles the corner cases where the entire text is replaced with itself(or transformed by developer) after each editing." + bug: "355137282" +} + +flag { name: "insert_mode_not_update_selection" namespace: "text" description: "Fix that InputService#onUpdateSelection is not called when insert mode gesture is performed." diff --git a/core/java/android/text/method/InsertModeTransformationMethod.java b/core/java/android/text/method/InsertModeTransformationMethod.java index 59b80f3a6468..6c6576f8888e 100644 --- a/core/java/android/text/method/InsertModeTransformationMethod.java +++ b/core/java/android/text/method/InsertModeTransformationMethod.java @@ -36,6 +36,7 @@ import android.view.View; import com.android.internal.util.ArrayUtils; import com.android.internal.util.Preconditions; +import com.android.text.flags.Flags; import java.lang.reflect.Array; @@ -171,9 +172,15 @@ public class InsertModeTransformationMethod implements TransformationMethod, Tex // The text change is before the highlight start, move the highlight start. mStart += diff; } else { - // The text change covers the highlight start. Extend the highlight start to the - // change start. This should be a rare case. - mStart = start; + if (Flags.insertModeHighlightRange()) { + // The text change covers the highlight start. Don't change the start except + // when it's out of range. + mStart = Math.min(mStart, s.length()); + } else { + // The text change covers the highlight start. Extend the highlight start to the + // change start. This should be a rare case. + mStart = start; + } } } @@ -181,9 +188,15 @@ public class InsertModeTransformationMethod implements TransformationMethod, Tex // The text change is before the highlight end, move the highlight end. mEnd += diff; } else if (start < mEnd) { - // The text change covers the highlight end. Extend the highlight end to the - // change end. This should be a rare case. - mEnd = start + count; + if (Flags.insertModeHighlightRange()) { + // The text change covers the highlight end. Don't change the end except when it's + // out of range. + mEnd = Math.min(mEnd, s.length()); + } else { + // The text change covers the highlight end. Extend the highlight end to the + // change end. This should be a rare case. + mEnd = start + count; + } } } diff --git a/core/tests/coretests/src/android/text/method/InsertModeTransformationMethodTest.java b/core/tests/coretests/src/android/text/method/InsertModeTransformationMethodTest.java index 2f336ab692f6..e2c19024a840 100644 --- a/core/tests/coretests/src/android/text/method/InsertModeTransformationMethodTest.java +++ b/core/tests/coretests/src/android/text/method/InsertModeTransformationMethodTest.java @@ -20,6 +20,10 @@ import static com.google.common.truth.Truth.assertThat; import android.content.Context; import android.platform.test.annotations.Presubmit; +import android.platform.test.annotations.RequiresFlagsDisabled; +import android.platform.test.annotations.RequiresFlagsEnabled; +import android.platform.test.flag.junit.CheckFlagsRule; +import android.platform.test.flag.junit.DeviceFlagsValueProvider; import android.text.SpannableString; import android.text.SpannableStringBuilder; import android.text.Spanned; @@ -30,7 +34,10 @@ import androidx.test.InstrumentationRegistry; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SmallTest; +import com.android.text.flags.Flags; + import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,6 +48,9 @@ public class InsertModeTransformationMethodTest { private static View sView; private static final String TEXT = "abc def"; + @Rule + public CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule(); + @BeforeClass public static void setupClass() { final Context context = InstrumentationRegistry.getTargetContext(); @@ -76,11 +86,13 @@ public class InsertModeTransformationMethodTest { } @Test + @RequiresFlagsDisabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) public void transformedText_charAt_editing() { transformedText_charAt_editing(false, "\n\n"); } @Test + @RequiresFlagsDisabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) public void transformedText_charAt_singleLine_editing() { transformedText_charAt_editing(true, "\uFFFD"); } @@ -132,6 +144,64 @@ public class InsertModeTransformationMethodTest { } @Test + @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) + public void transformedText_charAt_editing_stickyHighlightRange() { + transformedText_charAt_editing_stickyHighlightRange(false, "\n\n"); + } + + @Test + @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) + public void transformedText_charAt_singleLine_editing_stickyHighlightRange() { + transformedText_charAt_editing_stickyHighlightRange(true, "\uFFFD"); + } + + private void transformedText_charAt_editing_stickyHighlightRange(boolean singleLine, + String placeholder) { + final SpannableStringBuilder text = new SpannableStringBuilder(TEXT); + final InsertModeTransformationMethod transformationMethod = + new InsertModeTransformationMethod(3, singleLine, null); + final CharSequence transformedText = transformationMethod.getTransformation(text, sView); + // TransformationMethod is set on the original text as a TextWatcher in the TextView. + text.setSpan(transformationMethod, 0, text.length(), Spanned.SPAN_INCLUSIVE_EXCLUSIVE); + + assertCharSequence(transformedText, "abc" + placeholder + " def"); + + // original text is "abcxx def" after insertion. + text.insert(3, "xx"); + assertCharSequence(transformedText, "abcxx" + placeholder + " def"); + + // original text is "abcxx vvdef" after insertion. + text.insert(6, "vv"); + assertCharSequence(transformedText, "abcxx" + placeholder + " vvdef"); + + // original text is "abc vvdef" after deletion. + text.delete(3, 5); + assertCharSequence(transformedText, "abc" + placeholder + " vvdef"); + + // original text is "abc def" after deletion. + text.delete(4, 6); + assertCharSequence(transformedText, "abc" + placeholder + " def"); + + // original text is "abdef" after deletion. + // deletion range covers the placeholder's insertion point. It'll try to stay the same, + // which is still at index 3. + text.delete(2, 4); + assertCharSequence(transformedText, "abd" + placeholder + "ef"); + + // original text is "axxdef" after replace. + // this time the replaced range is ahead of the placeholder's insertion point. It updates to + // index 4. + text.replace(1, 2, "xx"); + assertCharSequence(transformedText, "axxd" + placeholder + "ef"); + + // original text is "ax" after replace. + // the deleted range covers the placeholder's insertion point. It tries to stay at index 4. + // However, 4 out of bounds now. So placeholder is inserted at the end of the string. + text.delete(2, 6); + assertCharSequence(transformedText, "ax" + placeholder); + } + + @Test public void transformedText_subSequence() { for (int offset = 0; offset < TEXT.length(); ++offset) { final InsertModeTransformationMethod transformationMethod = @@ -697,7 +767,7 @@ public class InsertModeTransformationMethodTest { } @Test - public void transformedText_getHighlightStartAndEnd_insertion_singleLine() { + public void transformedText_getHighlightStartAndEnd_singleLine_insertion() { transformedText_getHighlightStartAndEnd_insertion(true, "\uFDDD"); } @@ -751,16 +821,18 @@ public class InsertModeTransformationMethodTest { } @Test + @RequiresFlagsDisabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) public void transformedText_getHighlightStartAndEnd_deletion() { transformedText_getHighlightStartAndEnd_deletion(false, "\n\n"); } @Test - public void transformedText_getHighlightStartAndEnd_insertion_deletion() { + @RequiresFlagsDisabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) + public void transformedText_getHighlightStartAndEnd_singleLine_deletion() { transformedText_getHighlightStartAndEnd_deletion(true, "\uFDDD"); } - public void transformedText_getHighlightStartAndEnd_deletion(boolean singleLine, + private void transformedText_getHighlightStartAndEnd_deletion(boolean singleLine, String placeholder) { final SpannableStringBuilder text = new SpannableStringBuilder(TEXT); final InsertModeTransformationMethod transformationMethod = @@ -816,14 +888,93 @@ public class InsertModeTransformationMethodTest { assertThat(transformedText.getHighlightEnd()).isEqualTo(1 + placeholder.length()); } + @Test + @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) + public void transformedText_getHighlightStartAndEnd_deletion_stickyHighlightRange() { + transformedText_getHighlightStartAndEnd_deletion_stickyHighlightRange(false, "\n\n"); + } + + @Test + @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) + public void transformedText_getHighlightStartAndEnd_singleLine_deletion_stickyHighlightRange() { + transformedText_getHighlightStartAndEnd_deletion_stickyHighlightRange(true, "\uFDDD"); + } + + private void transformedText_getHighlightStartAndEnd_deletion_stickyHighlightRange( + boolean singleLine, String placeholder) { + final SpannableStringBuilder text = new SpannableStringBuilder(TEXT); + final InsertModeTransformationMethod transformationMethod = + new InsertModeTransformationMethod(3, singleLine, null); + final InsertModeTransformationMethod.TransformedText transformedText = + (InsertModeTransformationMethod.TransformedText) transformationMethod + .getTransformation(text, sView); + // TransformationMethod is set on the original text as a TextWatcher in the TextView. + text.setSpan(transformationMethod, 0, text.length(), Spanned.SPAN_INCLUSIVE_EXCLUSIVE); + + // note: the placeholder text is also highlighted. + assertThat(transformedText.getHighlightStart()).isEqualTo(3); + assertThat(transformedText.getHighlightEnd()).isEqualTo(3 + placeholder.length()); + + // original text is "abcxxxxxx def" after insertion. + // the placeholder is now inserted at index 9. + // the highlight start is still 3. + // the highlight end now is 9 + placeholder.length(). + text.insert(3, "xxxxxx"); + assertThat(transformedText.getHighlightStart()).isEqualTo(3); + assertThat(transformedText.getHighlightEnd()).isEqualTo(9 + placeholder.length()); + + // original text is "abxxxxxx def" after deletion. + // the placeholder is now inserted at index 6. + // the highlight start is 2, since the deletion happens before the highlight range. + // the highlight end now is 8 + placeholder.length(). + text.delete(2, 3); + assertThat(transformedText.getHighlightStart()).isEqualTo(2); + assertThat(transformedText.getHighlightEnd()).isEqualTo(8 + placeholder.length()); + + // original text is "abxxx def" after deletion. + // the placeholder is now inserted at index 5. + // the highlight start is still 2, since the deletion happens in the highlight range. + // the highlight end now is 5 + placeholder.length(). + text.delete(2, 5); + assertThat(transformedText.getHighlightStart()).isEqualTo(2); + assertThat(transformedText.getHighlightEnd()).isEqualTo(5 + placeholder.length()); + + // original text is "abxxx d" after deletion. + // the placeholder is now inserted at index 5. + // the highlight start is still 2, since the deletion happens after the highlight range. + // the highlight end now is still 5 + placeholder.length(). + text.delete(7, 9); + assertThat(transformedText.getHighlightStart()).isEqualTo(2); + assertThat(transformedText.getHighlightEnd()).isEqualTo(5 + placeholder.length()); + + // original text is "axx d" after deletion. + // the placeholder is now inserted at index 3. + // the highlight start is at 2, since the deletion range covers the start. + // the highlight end is 3 + placeholder.length(). + text.delete(1, 3); + assertThat(transformedText.getHighlightStart()).isEqualTo(2); + assertThat(transformedText.getHighlightEnd()).isEqualTo(3 + placeholder.length()); + + // original text is "ax" after deletion. + // the placeholder is now inserted at index 2. + // the highlight start is at 2. + // the highlight end is 2 + placeholder.length(). It wants to stay at 3, but it'll be out + // of bounds, so it'll be 2 instead. + text.delete(2, 5); + assertThat(transformedText.getHighlightStart()).isEqualTo(2); + assertThat(transformedText.getHighlightEnd()).isEqualTo(2 + placeholder.length()); + } + @Test + @RequiresFlagsDisabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) public void transformedText_getHighlightStartAndEnd_replace() { transformedText_getHighlightStartAndEnd_replace(false, "\n\n"); } @Test - public void transformedText_getHighlightStartAndEnd_insertion__replace() { + @RequiresFlagsDisabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) + public void transformedText_getHighlightStartAndEnd_singleLine_replace() { transformedText_getHighlightStartAndEnd_replace(true, "\uFDDD"); } @@ -908,6 +1059,99 @@ public class InsertModeTransformationMethodTest { assertThat(transformedText.getHighlightEnd()).isEqualTo(3 + placeholder.length()); } + @Test + @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) + public void transformedText_getHighlightStartAndEnd_replace_stickyHighlightRange() { + transformedText_getHighlightStartAndEnd_replace_stickyHighlightRange(false, "\n\n"); + } + + @Test + @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_HIGHLIGHT_RANGE) + public void transformedText_getHighlightStartAndEnd_singleLine_replace_stickyHighlightRange() { + transformedText_getHighlightStartAndEnd_replace_stickyHighlightRange(true, "\uFDDD"); + } + + private void transformedText_getHighlightStartAndEnd_replace_stickyHighlightRange( + boolean singleLine, String placeholder) { + final SpannableStringBuilder text = new SpannableStringBuilder(TEXT); + final InsertModeTransformationMethod transformationMethod = + new InsertModeTransformationMethod(3, singleLine, null); + final InsertModeTransformationMethod.TransformedText transformedText = + (InsertModeTransformationMethod.TransformedText) transformationMethod + .getTransformation(text, sView); + // TransformationMethod is set on the original text as a TextWatcher in the TextView. + text.setSpan(transformationMethod, 0, text.length(), Spanned.SPAN_INCLUSIVE_EXCLUSIVE); + + // note: the placeholder text is also highlighted. + assertThat(transformedText.getHighlightStart()).isEqualTo(3); + assertThat(transformedText.getHighlightEnd()).isEqualTo(3 + placeholder.length()); + + // original text is "abcxxxxxx def" after insertion. + // the placeholder is now inserted at index 9. + // the highlight start is still 3. + // the highlight end now is 9 + placeholder.length(). + text.insert(3, "xxxxxx"); + assertThat(transformedText.getHighlightStart()).isEqualTo(3); + assertThat(transformedText.getHighlightEnd()).isEqualTo(9 + placeholder.length()); + + // original text is "abvvxxxxxx def" after replace. + // the replacement happens before the highlight range; highlight range is offset by 1 + // the placeholder is now inserted at index 10, + // the highlight start is 4. + // the highlight end is 10 + placeholder.length(). + text.replace(2, 3, "vv"); + assertThat(transformedText.getHighlightStart()).isEqualTo(4); + assertThat(transformedText.getHighlightEnd()).isEqualTo(10 + placeholder.length()); + + // original text is "abvvxxx def" after replace. + // the replacement happens in the highlight range; highlight end is offset by -3 + // the placeholder is now inserted at index 7, + // the highlight start is still 4. + // the highlight end is 7 + placeholder.length(). + text.replace(5, 9, "x"); + assertThat(transformedText.getHighlightStart()).isEqualTo(4); + assertThat(transformedText.getHighlightEnd()).isEqualTo(7 + placeholder.length()); + + // original text is "abvvxxxvvv" after replace. + // the replacement happens after the highlight range; highlight is not changed + // the placeholder is now inserted at index 7, + // the highlight start is still 4. + // the highlight end is 7 + placeholder.length(). + text.replace(7, 11, "vvv"); + assertThat(transformedText.getHighlightStart()).isEqualTo(4); + assertThat(transformedText.getHighlightEnd()).isEqualTo(7 + placeholder.length()); + + // original text is "abxxxxvvv" after replace. + // the replacement covers the highlight start; highlight start stays the same; + // highlight end is offset by -1 + // the placeholder is now inserted at index 6, + // the highlight start is 4. + // the highlight end is 6 + placeholder.length(). + text.replace(2, 5, "xx"); + assertThat(transformedText.getHighlightStart()).isEqualTo(4); + assertThat(transformedText.getHighlightEnd()).isEqualTo(6 + placeholder.length()); + + // original text is "abxxxxxvv" after replace. + // the replacement covers the highlight end; highlight end stays the same; + // highlight start stays the same + // the placeholder is now inserted at index 6, + // the highlight start is 2. + // the highlight end is 6 + placeholder.length(). + text.replace(5, 7, "xx"); + assertThat(transformedText.getHighlightStart()).isEqualTo(4); + assertThat(transformedText.getHighlightEnd()).isEqualTo(6 + placeholder.length()); + + // original text is "axxv" after replace. + // the replacement covers the highlight range; highlight start stays the same. + // highlight end shrink to the text length. + // the placeholder is now inserted at index 3, + // the highlight start is 2. + // the highlight end is 4 + placeholder.length(). + text.replace(1, 8, "xx"); + assertThat(transformedText.getHighlightStart()).isEqualTo(4); + assertThat(transformedText.getHighlightEnd()).isEqualTo(4 + placeholder.length()); + } + private static <T> void assertNextSpanTransition(Spanned spanned, int[] transitions, Class<T> type) { int currentTransition = 0; |