From cfa26a0ae73045646e6ec674602c5304417aaff3 Mon Sep 17 00:00:00 2001 From: Tony Mak Date: Thu, 10 Feb 2022 20:08:31 +0000 Subject: [PATCH] Fix the empty selection issue when onCreateActionMode returns false One of the optimizations ag/12911059 did was calling SelectionModifierCursorController.show() before startActionModeInternal(). The rationale was that if we start the action mode first, SelectionModifierCursorController.show() would end up invalidating the action mode twice unnecessarily, once for each handle. However, with this optimization, we are calling SelectionModifierCursorController.show() even when onCreateActionMode returns false. Reverted this particular optimization to fix the issue. Added a test which was failing without this fix but passing with it. Fixes: 199380016 Fixes: 214341747 Test: atest TextViewActivityTest Merged-In: I793f76a23978cbbbbde2d16e8a522615174bcdd5 Change-Id: I793f76a23978cbbbbde2d16e8a522615174bcdd5 (cherry picked from commit 11bd64482249c9257ee22390de15f69f9c834247) --- .../widget/SelectionActionModeHelper.java | 10 +++--- .../android/widget/TextViewActivityTest.java | 36 +++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/core/java/android/widget/SelectionActionModeHelper.java b/core/java/android/widget/SelectionActionModeHelper.java index 2d1f17a420fa..a0ec48bc6beb 100644 --- a/core/java/android/widget/SelectionActionModeHelper.java +++ b/core/java/android/widget/SelectionActionModeHelper.java @@ -297,12 +297,12 @@ public final class SelectionActionModeHelper { } else { mTextClassification = null; } - final SelectionModifierCursorController controller = mEditor.getSelectionController(); - if (controller != null - && (mTextView.isTextSelectable() || mTextView.isTextEditable())) { - controller.show(); - } if (mEditor.startActionModeInternal(actionMode)) { + final SelectionModifierCursorController controller = mEditor.getSelectionController(); + if (controller != null + && (mTextView.isTextSelectable() || mTextView.isTextEditable())) { + controller.show(); + } if (result != null) { switch (actionMode) { case Editor.TextActionMode.SELECTION: diff --git a/core/tests/coretests/src/android/widget/TextViewActivityTest.java b/core/tests/coretests/src/android/widget/TextViewActivityTest.java index 40ef04a5e369..152992cc57a4 100644 --- a/core/tests/coretests/src/android/widget/TextViewActivityTest.java +++ b/core/tests/coretests/src/android/widget/TextViewActivityTest.java @@ -17,6 +17,7 @@ package android.widget; import static android.widget.espresso.CustomViewActions.longPressAtRelativeCoordinates; +import static android.widget.espresso.DragHandleUtils.assertNoSelectionHandles; import static android.widget.espresso.DragHandleUtils.onHandleView; import static android.widget.espresso.FloatingToolbarEspressoUtils.assertFloatingToolbarContainsItem; import static android.widget.espresso.FloatingToolbarEspressoUtils.assertFloatingToolbarDoesNotContainItem; @@ -425,6 +426,41 @@ public class TextViewActivityTest { assertEquals(latestItem[0], clickedItem[0]); } + @Test + public void testSelectionOnCreateActionModeReturnsFalse() throws Throwable { + final String text = "hello world"; + mActivityRule.runOnUiThread(() -> { + final TextView textView = mActivity.findViewById(R.id.textview); + textView.setText(text); + textView.setCustomSelectionActionModeCallback( + new ActionMode.Callback() { + @Override + public boolean onCreateActionMode(ActionMode mode, Menu menu) { + return false; + } + + @Override + public boolean onPrepareActionMode(ActionMode mode, Menu menu) { + return false; + } + + @Override + public boolean onActionItemClicked(ActionMode mode, MenuItem item) { + return false; + } + + + @Override + public void onDestroyActionMode(ActionMode mode) { + } + }); + }); + mInstrumentation.waitForIdleSync(); + onView(withId(R.id.textview)).perform(longPressOnTextAtIndex(text.indexOf("d"))); + mInstrumentation.waitForIdleSync(); + assertNoSelectionHandles(); + } + @Test public void testSelectionRemovedWhenNonselectableTextLosesFocus() throws Throwable { final TextLinks.TextLink textLink = addLinkifiedTextToTextView(R.id.nonselectable_textview);