From 520969191b37ddfca441b8065543fd0dee7dd74e Mon Sep 17 00:00:00 2001 From: Abodunrinwa Toki Date: Wed, 21 Mar 2018 23:14:42 +0000 Subject: [PATCH] Fix random SmartLinkify-related TextView bugs. 1. Preserve selection when the TC times out. (See: SelectionActionModeHelper) 2. Fix highlight/toolbar flicker when tapping on a smart link. - Highlight flicker happening because we reset the selection while in the process of starting a link action mode. i.e. onLinkDown: show highlight onLinkUp: start the link action mode asynchronously onLinkUp: reset the selection to an insertion cursor* onLinkActionModeStarted: reset the highlight *Fix: Don't reset selection while starting a link action mode. - Toolbar flicker happening because the toolbar positions itself over the current selection. The way link highlights have traditionally been done is to set the selection to the links bounds* *Fix: Hide the toolbar for a few milliseconds when changing selection for smoother transition. 3. Fix Paste menu overriding link action mode toolbar after a recent "Copy" action. The Paste menu appearing is a feature. Whenever the user inserts a cursor after just copying some text, we show the Paste menu as a way to make it easy for the user to select the text. Because of the problem described in (2) above, changing the selection to an insertion triggers the Paste menu feature. Fixing (2) fixes this. 4. Fix IME popping up on non-selectable + focusable TextViews. See: imm.showSoftInput(...) in Editor. And see comment in the code around that. We should only pop up the IME for editable text. Fixes: 73872461 Fixes: 75985239 Fixes: 76011461 Test: bit CtsWidgetTestCases:android.widget.cts.TextViewTest Test: bit FrameworksCoreTests:android.widget.TextViewActivityTest Change-Id: If9ddb7f4e6d4db480ba4a495a22f7f2924ab937e --- .../text/method/LinkMovementMethod.java | 11 ++++++- core/java/android/widget/Editor.java | 32 ++++++++++++------- .../widget/SelectionActionModeHelper.java | 31 ++++++++++++------ core/java/android/widget/TextView.java | 9 +++++- 4 files changed, 60 insertions(+), 23 deletions(-) diff --git a/core/java/android/text/method/LinkMovementMethod.java b/core/java/android/text/method/LinkMovementMethod.java index 31ed549260a09..f332358893884 100644 --- a/core/java/android/text/method/LinkMovementMethod.java +++ b/core/java/android/text/method/LinkMovementMethod.java @@ -16,6 +16,7 @@ package android.text.method; +import android.os.Build; import android.text.Layout; import android.text.NoCopySpan; import android.text.Selection; @@ -35,6 +36,8 @@ public class LinkMovementMethod extends ScrollingMovementMethod { private static final int UP = 2; private static final int DOWN = 3; + private static final int HIDE_FLOATING_TOOLBAR_DELAY_MS = 200; + @Override public boolean canSelectArbitrarily() { return true; @@ -65,7 +68,7 @@ public class LinkMovementMethod extends ScrollingMovementMethod { return super.up(widget, buffer); } - + @Override protected boolean down(TextView widget, Spannable buffer) { if (action(DOWN, widget, buffer)) { @@ -215,6 +218,12 @@ public class LinkMovementMethod extends ScrollingMovementMethod { if (action == MotionEvent.ACTION_UP) { links[0].onClick(widget); } else if (action == MotionEvent.ACTION_DOWN) { + if (widget.getContext().getApplicationInfo().targetSdkVersion + > Build.VERSION_CODES.O_MR1) { + // Selection change will reposition the toolbar. Hide it for a few ms for a + // smoother transition. + widget.hideFloatingToolbar(HIDE_FLOATING_TOOLBAR_DELAY_MS); + } Selection.setSelection(buffer, buffer.getSpanStart(links[0]), buffer.getSpanEnd(links[0])); diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java index 92285c77e61cd..3775835aa6d83 100644 --- a/core/java/android/widget/Editor.java +++ b/core/java/android/widget/Editor.java @@ -289,6 +289,7 @@ public class Editor { boolean mShowSoftInputOnFocus = true; private boolean mPreserveSelection; private boolean mRestartActionModeOnNextRefresh; + private boolean mRequestingLinkActionMode; private SelectionActionModeHelper mSelectionActionModeHelper; @@ -2127,6 +2128,7 @@ public class Editor { return; } stopTextActionMode(); + mRequestingLinkActionMode = true; getSelectionActionModeHelper().startLinkActionModeAsync(start, end); } @@ -2212,7 +2214,9 @@ public class Editor { mTextActionMode = mTextView.startActionMode(actionModeCallback, ActionMode.TYPE_FLOATING); final boolean selectionStarted = mTextActionMode != null; - if (selectionStarted && !mTextView.isTextSelectable() && mShowSoftInputOnFocus) { + if (selectionStarted + && mTextView.isTextEditable() && !mTextView.isTextSelectable() + && mShowSoftInputOnFocus) { // Show the IME to be able to replace text, except when selecting non editable text. final InputMethodManager imm = InputMethodManager.peekInstance(); if (imm != null) { @@ -2322,10 +2326,14 @@ public class Editor { if (!selectAllGotFocus && text.length() > 0) { // Move cursor final int offset = mTextView.getOffsetForPosition(event.getX(), event.getY()); - Selection.setSelection((Spannable) text, offset); - if (mSpellChecker != null) { - // When the cursor moves, the word that was typed may need spell check - mSpellChecker.onSelectionChanged(); + + final boolean shouldInsertCursor = !mRequestingLinkActionMode; + if (shouldInsertCursor) { + Selection.setSelection((Spannable) text, offset); + if (mSpellChecker != null) { + // When the cursor moves, the word that was typed may need spell check + mSpellChecker.onSelectionChanged(); + } } if (!extractedTextModeWillBeStarted()) { @@ -2335,16 +2343,17 @@ public class Editor { mTextView.removeCallbacks(mInsertionActionModeRunnable); } - mShowSuggestionRunnable = new Runnable() { - public void run() { - replace(); - } - }; + mShowSuggestionRunnable = this::replace; + // removeCallbacks is performed on every touch mTextView.postDelayed(mShowSuggestionRunnable, ViewConfiguration.getDoubleTapTimeout()); } else if (hasInsertionController()) { - getInsertionController().show(); + if (shouldInsertCursor) { + getInsertionController().show(); + } else { + getInsertionController().hide(); + } } } } @@ -4170,6 +4179,7 @@ public class Editor { } mAssistClickHandlers.clear(); + mRequestingLinkActionMode = false; } @Override diff --git a/core/java/android/widget/SelectionActionModeHelper.java b/core/java/android/widget/SelectionActionModeHelper.java index be8c34c53ae69..05204d03f60aa 100644 --- a/core/java/android/widget/SelectionActionModeHelper.java +++ b/core/java/android/widget/SelectionActionModeHelper.java @@ -71,7 +71,7 @@ public final class SelectionActionModeHelper { private final TextClassificationHelper mTextClassificationHelper; private final TextClassificationConstants mTextClassificationSettings; - private TextClassification mTextClassification; + @Nullable private TextClassification mTextClassification; private AsyncTask mTextClassificationAsyncTask; private final SelectionTracker mSelectionTracker; @@ -124,7 +124,8 @@ public final class SelectionActionModeHelper { : mTextClassificationHelper::classifyText, mSmartSelectSprite != null ? this::startSelectionActionModeWithSmartSelectAnimation - : this::startSelectionActionMode) + : this::startSelectionActionMode, + mTextClassificationHelper::getOriginalSelection) .execute(); } } @@ -143,7 +144,8 @@ public final class SelectionActionModeHelper { mTextView, mTextClassificationHelper.getTimeoutDuration(), mTextClassificationHelper::classifyText, - this::startLinkActionMode) + this::startLinkActionMode, + mTextClassificationHelper::getOriginalSelection) .execute(); } } @@ -158,7 +160,8 @@ public final class SelectionActionModeHelper { mTextView, mTextClassificationHelper.getTimeoutDuration(), mTextClassificationHelper::classifyText, - this::invalidateActionMode) + this::invalidateActionMode, + mTextClassificationHelper::getOriginalSelection) .execute(); } } @@ -246,7 +249,7 @@ public final class SelectionActionModeHelper { mTextView.invalidate(); } mTextClassification = result.mClassification; - } else if (actionMode == Editor.TextActionMode.TEXT_LINK) { + } else if (result != null && actionMode == Editor.TextActionMode.TEXT_LINK) { mTextClassification = result.mClassification; } else { mTextClassification = null; @@ -822,6 +825,7 @@ public final class SelectionActionModeHelper { private final int mTimeOutDuration; private final Supplier mSelectionResultSupplier; private final Consumer mSelectionResultCallback; + private final Supplier mTimeOutResultSupplier; private final TextView mTextView; private final String mOriginalText; @@ -830,16 +834,19 @@ public final class SelectionActionModeHelper { * @param timeOut time in milliseconds to timeout the query if it has not completed * @param selectionResultSupplier fetches the selection results. Runs on a background thread * @param selectionResultCallback receives the selection results. Runs on the UiThread + * @param timeOutResultSupplier default result if the task times out */ TextClassificationAsyncTask( @NonNull TextView textView, int timeOut, @NonNull Supplier selectionResultSupplier, - @NonNull Consumer selectionResultCallback) { + @NonNull Consumer selectionResultCallback, + @NonNull Supplier timeOutResultSupplier) { super(textView != null ? textView.getHandler() : null); mTextView = Preconditions.checkNotNull(textView); mTimeOutDuration = timeOut; mSelectionResultSupplier = Preconditions.checkNotNull(selectionResultSupplier); mSelectionResultCallback = Preconditions.checkNotNull(selectionResultCallback); + mTimeOutResultSupplier = Preconditions.checkNotNull(timeOutResultSupplier); // Make a copy of the original text. mOriginalText = getText(mTextView).toString(); } @@ -863,7 +870,7 @@ public final class SelectionActionModeHelper { private void onTimeOut() { if (getStatus() == Status.RUNNING) { - onPostExecute(null); + onPostExecute(mTimeOutResultSupplier.get()); } cancel(true); } @@ -963,6 +970,10 @@ public final class SelectionActionModeHelper { return performClassification(selection); } + public SelectionResult getOriginalSelection() { + return new SelectionResult(mSelectionStart, mSelectionEnd, null, null); + } + /** * Maximum time (in milliseconds) to wait for a textclassifier result before timing out. */ @@ -1025,14 +1036,14 @@ public final class SelectionActionModeHelper { private static final class SelectionResult { private final int mStart; private final int mEnd; - private final TextClassification mClassification; + @Nullable private final TextClassification mClassification; @Nullable private final TextSelection mSelection; SelectionResult(int start, int end, - TextClassification classification, @Nullable TextSelection selection) { + @Nullable TextClassification classification, @Nullable TextSelection selection) { mStart = start; mEnd = end; - mClassification = Preconditions.checkNotNull(classification); + mClassification = classification; mSelection = selection; } } diff --git a/core/java/android/widget/TextView.java b/core/java/android/widget/TextView.java index 1e2d18c3ad838..da379fa6c61f3 100644 --- a/core/java/android/widget/TextView.java +++ b/core/java/android/widget/TextView.java @@ -11558,6 +11558,13 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener } } + /** @hide */ + public void hideFloatingToolbar(int durationMs) { + if (mEditor != null) { + mEditor.hideFloatingToolbar(durationMs); + } + } + boolean canUndo() { return mEditor != null && mEditor.canUndo(); } @@ -11652,7 +11659,7 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener boolean selectAllText() { if (mEditor != null) { // Hide the toolbar before changing the selection to avoid flickering. - mEditor.hideFloatingToolbar(FLOATING_TOOLBAR_SELECT_ALL_REFRESH_DELAY); + hideFloatingToolbar(FLOATING_TOOLBAR_SELECT_ALL_REFRESH_DELAY); } final int length = mText.length(); Selection.setSelection((Spannable) mText, 0, length);