From c477b5830af5f9e87069fae4ff87a2a6b3e275fb Mon Sep 17 00:00:00 2001 From: Keisuke Kuroyanagi Date: Tue, 15 Mar 2016 15:38:40 +0900 Subject: [PATCH] Stop automatically creating selection action mode. With Ic025c109539c3b5963, selection action mode is created always when selection is made. It causes distraction in some cases. This CL fixes this issue. This CL stops starting selection action mode when action mode is currently not active and not intended to restart. Bug: 27536744 Bug: 27551819 Change-Id: I94ee66864000934a21ef0d18c1d71429c67114fa --- core/java/android/widget/Editor.java | 56 ++++++++-------- core/java/android/widget/TextView.java | 4 ++ .../android/widget/TextViewActivityTest.java | 66 +++++++++++++++++++ 3 files changed, 97 insertions(+), 29 deletions(-) diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java index 3b6ba3a758a1b..940dddd74f396 100644 --- a/core/java/android/widget/Editor.java +++ b/core/java/android/widget/Editor.java @@ -216,6 +216,7 @@ public class Editor { boolean mInBatchEditControllers; boolean mShowSoftInputOnFocus = true; private boolean mPreserveSelection; + private boolean mRestartActionModeOnNextRefresh; boolean mTemporaryDetach; boolean mIsBeingLongClicked; @@ -380,9 +381,8 @@ public class Editor { updateSpellCheckSpans(0, mTextView.getText().length(), true /* create the spell checker if needed */); - if (mTextView.getSelectionStart() != mTextView.getSelectionEnd()) { - // We had an active selection from before, start the selection mode. - startSelectionActionMode(); + if (mTextView.hasSelection()) { + refreshTextActionMode(); } getPositionListener().addSubscriber(mCursorAnchorInfoNotifier, true); @@ -1283,7 +1283,7 @@ public class Editor { } final InputMethodManager imm = InputMethodManager.peekInstance(); if (mTextView.hasSelection() && !extractedTextModeWillBeStarted()) { - startSelectionActionMode(); + refreshTextActionMode(); } } else { if (mBlink != null) { @@ -1846,6 +1846,7 @@ public class Editor { void refreshTextActionMode() { if (extractedTextModeWillBeStarted()) { + mRestartActionModeOnNextRefresh = false; return; } final boolean hasSelection = mTextView.hasSelection(); @@ -1854,12 +1855,19 @@ public class Editor { if ((selectionController != null && selectionController.isCursorBeingModified()) || (insertionController != null && insertionController.isCursorBeingModified())) { // ActionMode should be managed by the currently active cursor controller. + mRestartActionModeOnNextRefresh = false; return; } if (hasSelection) { - if (mTextActionMode == null || selectionController == null - || !selectionController.isActive()) { - // Avoid dismissing the selection if it exists. + hideInsertionPointCursorController(); + if (mTextActionMode == null) { + if (mRestartActionModeOnNextRefresh || mTextView.isInExtractedMode()) { + // To avoid distraction, newly start action mode only when selection action + // mode is being restarted or in full screen extracted mode. + startSelectionActionMode(); + } + } else if (selectionController == null || !selectionController.isActive()) { + // Insertion action mode is active. Avoid dismissing the selection. stopTextActionModeWithPreservingSelection(); startSelectionActionMode(); } else { @@ -1874,6 +1882,7 @@ public class Editor { mTextActionMode.invalidateContentRect(); } } + mRestartActionModeOnNextRefresh = false; } /** @@ -1904,11 +1913,12 @@ public class Editor { * * @return true if the selection mode was actually started. */ - private boolean startSelectionActionMode() { + boolean startSelectionActionMode() { boolean selectionStarted = startSelectionActionModeInternal(); if (selectionStarted) { getSelectionController().show(); } + mRestartActionModeOnNextRefresh = false; return selectionStarted; } @@ -2112,6 +2122,9 @@ public class Editor { } private void stopTextActionModeWithPreservingSelection() { + if (mTextActionMode != null) { + mRestartActionModeOnNextRefresh = true; + } mPreserveSelection = true; stopTextActionMode(); mPreserveSelection = false; @@ -3661,6 +3674,8 @@ public class Editor { @Override public void onDestroyActionMode(ActionMode mode) { + // Clear mTextActionMode not to recursively destroy action mode by clearing selection. + mTextActionMode = null; Callback customCallback = getCustomCallback(); if (customCallback != null) { customCallback.onDestroyActionMode(mode); @@ -3679,8 +3694,6 @@ public class Editor { if (mSelectionModifierCursorController != null) { mSelectionModifierCursorController.hide(); } - - mTextActionMode = null; } @Override @@ -5083,27 +5096,12 @@ public class Editor { // No longer dragging to select text, let the parent intercept events. mTextView.getParent().requestDisallowInterceptTouchEvent(false); - int startOffset = mTextView.getSelectionStart(); - int endOffset = mTextView.getSelectionEnd(); - - // Since we don't let drag handles pass once they're visible, we need to - // make sure the start / end locations are correct because the user *can* - // switch directions during the initial drag. - if (endOffset < startOffset) { - int tmp = endOffset; - endOffset = startOffset; - startOffset = tmp; - - // Also update the selection with the right offsets in this case. - Selection.setSelection((Spannable) mTextView.getText(), - startOffset, endOffset); - } - if (startOffset != endOffset) { - startSelectionActionMode(); - } - // No longer the first dragging motion, reset. resetDragAcceleratorState(); + + if (mTextView.hasSelection()) { + startSelectionActionMode(); + } break; } } diff --git a/core/java/android/widget/TextView.java b/core/java/android/widget/TextView.java index e971f865bf455..fbedbdac9065a 100644 --- a/core/java/android/widget/TextView.java +++ b/core/java/android/widget/TextView.java @@ -9212,6 +9212,10 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener } if (start >= 0 && start <= end && end <= text.length()) { Selection.setSelection((Spannable) text, start, end); + // Make sure selection mode is engaged. + if (mEditor != null) { + mEditor.startSelectionActionMode(); + } return true; } } diff --git a/core/tests/coretests/src/android/widget/TextViewActivityTest.java b/core/tests/coretests/src/android/widget/TextViewActivityTest.java index 844eadbae692b..4a4727fd891f3 100644 --- a/core/tests/coretests/src/android/widget/TextViewActivityTest.java +++ b/core/tests/coretests/src/android/widget/TextViewActivityTest.java @@ -47,6 +47,8 @@ import com.android.frameworks.coretests.R; import android.support.test.espresso.action.EspressoKey; import android.test.ActivityInstrumentationTestCase2; import android.test.suitebuilder.annotation.SmallTest; +import android.text.Selection; +import android.text.Spannable; import android.view.KeyEvent; import static org.hamcrest.Matchers.anyOf; @@ -534,4 +536,68 @@ public class TextViewActivityTest extends ActivityInstrumentationTestCase2 Selection.setSelection((Spannable) textView.getText(), 0, 3)); + getInstrumentation().waitForIdleSync(); + sleepForFloatingToolbarPopup(); + // Don't automatically start action mode. + assertFloatingToolbarIsNotDisplayed(); + // Make sure that "Select All" is included in the selection action mode when the entire text + // is not selected. + onView(withId(R.id.textview)).perform(doubleClickOnTextAtIndex(text.indexOf('e'))); + sleepForFloatingToolbarPopup(); + assertFloatingToolbarIsDisplayed(); + // Changing the selection range by API should not interrupt the selection action mode. + textView.post(() -> Selection.setSelection((Spannable) textView.getText(), 0, 3)); + getInstrumentation().waitForIdleSync(); + sleepForFloatingToolbarPopup(); + assertFloatingToolbarIsDisplayed(); + assertFloatingToolbarContainsItem( + getActivity().getString(com.android.internal.R.string.selectAll)); + // Make sure that "Select All" is no longer included when the entire text is selected by + // API. + textView.post( + () -> Selection.setSelection((Spannable) textView.getText(), 0, text.length())); + getInstrumentation().waitForIdleSync(); + sleepForFloatingToolbarPopup(); + assertFloatingToolbarIsDisplayed(); + assertFloatingToolbarDoesNotContainItem( + getActivity().getString(com.android.internal.R.string.selectAll)); + // Make sure that shrinking the selection range to cursor (an empty range) by API + // terminates selection action mode and does not trigger the insertion action mode. + textView.post(() -> Selection.setSelection((Spannable) textView.getText(), 0)); + getInstrumentation().waitForIdleSync(); + sleepForFloatingToolbarPopup(); + assertFloatingToolbarIsNotDisplayed(); + // Make sure that user click can trigger the insertion action mode. + onView(withId(R.id.textview)).perform(clickOnTextAtIndex(text.length())); + onHandleView(com.android.internal.R.id.insertion_handle).perform(click()); + sleepForFloatingToolbarPopup(); + assertFloatingToolbarIsDisplayed(); + // Make sure that an existing insertion action mode keeps alive after the insertion point is + // moved by API. + textView.post(() -> Selection.setSelection((Spannable) textView.getText(), 0)); + getInstrumentation().waitForIdleSync(); + sleepForFloatingToolbarPopup(); + assertFloatingToolbarIsDisplayed(); + assertFloatingToolbarDoesNotContainItem( + getActivity().getString(com.android.internal.R.string.copy)); + // Make sure that selection action mode is started after selection is created by API when + // insertion action mode is active. + textView.post( + () -> Selection.setSelection((Spannable) textView.getText(), 1, text.length())); + getInstrumentation().waitForIdleSync(); + sleepForFloatingToolbarPopup(); + assertFloatingToolbarIsDisplayed(); + assertFloatingToolbarContainsItem( + getActivity().getString(com.android.internal.R.string.copy)); + } }