Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f299fc03 authored by Abodunrinwa Toki's avatar Abodunrinwa Toki
Browse files

End the TC session on terminal selection event actions 

This regressed when introducing TC sessions in
I3c9ceea0863099fc4f0a5ce5e823c648ee9c4521
When the user triggers a terminal selection event such as "Copy",
we should immediately end the session instead of waiting for the
"Abandon" event (i.e. selection dismissed) to be included in the
logs. Terminal selection events implicitly dismiss a selection and
we'd rather distiguish between an actual "selection dismiss" from
one that happened because of a "terminal" selection event.

This cl also removes the "*" marker used to distinguish the new
logging from the old ones. The code for the old logging has already
been deleted so no more need for a marker.

Bug: 78541105
Test: bit CtsWidgetTestCases:android.widget.cts.TextViewTest
Test: bit FrameworksCoreTests:android.widget.TextViewActivityTest
Change-Id: Iac7d45dbc63e7076683742bd045766a1d927cfc9
parent 97abc762

core/java/android/widget/SelectionActionModeHelper.java

+33 −23
Original line number Diff line number Diff line
@@ -636,7 +636,7 @@ public final class SelectionActionModeHelper { 
                            mSelectionStart, mSelectionEnd,

                            SelectionEvent.ACTION_ABANDON, null /* classification */);

                    mSelectionStart = mSelectionEnd = -1;

                    mTextView.getTextClassificationSession().destroy();

                    mLogger.endTextClassificationSession();

                    mIsPending = false;

                }

            }
@@ -702,8 +702,10 @@ public final class SelectionActionModeHelper { 
                mTokenIterator.setText(mText);

                mStartIndex = index;

                mClassificationSession = classificationSession;

                if (hasActiveClassificationSession()) {

                    mClassificationSession.onSelectionEvent(

                            SelectionEvent.createSelectionStartedEvent(invocationMethod, 0));

                }

            } catch (Exception e) {

                // Avoid crashes due to logging.

                Log.e(LOG_TAG, "" + e.getMessage(), e);
@@ -713,23 +715,19 @@ public final class SelectionActionModeHelper { 
        public void logSelectionModified(int start, int end,

                @Nullable TextClassification classification, @Nullable TextSelection selection) {

            try {

                if (hasActiveClassificationSession()) {

                    Preconditions.checkArgumentInRange(start, 0, mText.length(), "start");

                    Preconditions.checkArgumentInRange(end, start, mText.length(), "end");

                    int[] wordIndices = getWordDelta(start, end);

                    if (selection != null) {

                    if (mClassificationSession != null) {

                        mClassificationSession.onSelectionEvent(

                                SelectionEvent.createSelectionModifiedEvent(

                                        wordIndices[0], wordIndices[1], selection));

                    }

                    } else if (classification != null) {

                    if (mClassificationSession != null) {

                        mClassificationSession.onSelectionEvent(

                                SelectionEvent.createSelectionModifiedEvent(

                                        wordIndices[0], wordIndices[1], classification));

                    }

                    } else {

                    if (mClassificationSession != null) {

                        mClassificationSession.onSelectionEvent(

                                SelectionEvent.createSelectionModifiedEvent(

                                        wordIndices[0], wordIndices[1]));
@@ -746,21 +744,23 @@ public final class SelectionActionModeHelper { 
                @SelectionEvent.ActionType int action,

                @Nullable TextClassification classification) {

            try {

                if (hasActiveClassificationSession()) {

                    Preconditions.checkArgumentInRange(start, 0, mText.length(), "start");

                    Preconditions.checkArgumentInRange(end, start, mText.length(), "end");

                    int[] wordIndices = getWordDelta(start, end);

                    if (classification != null) {

                    if (mClassificationSession != null) {

                        mClassificationSession.onSelectionEvent(

                                SelectionEvent.createSelectionActionEvent(

                                        wordIndices[0], wordIndices[1], action, classification));

                    }

                                        wordIndices[0], wordIndices[1], action,

                                        classification));

                    } else {

                    if (mClassificationSession != null) {

                        mClassificationSession.onSelectionEvent(

                                SelectionEvent.createSelectionActionEvent(

                                        wordIndices[0], wordIndices[1], action));

                    }

                    if (SelectionEvent.isTerminal(action)) {

                        endTextClassificationSession();

                    }

                }

            } catch (Exception e) {

                // Avoid crashes due to logging.
@@ -772,6 +772,16 @@ public final class SelectionActionModeHelper { 
            return mEditTextLogger;

        }



        public void endTextClassificationSession() {

            if (hasActiveClassificationSession()) {

                mClassificationSession.destroy();

            }

        }



        private boolean hasActiveClassificationSession() {

            return mClassificationSession != null && !mClassificationSession.isDestroyed();

        }



        private int[] getWordDelta(int start, int end) {

            int[] wordIndices = new int[2];

core/java/android/widget/TextView.java

+1 −4
Original line number Diff line number Diff line
@@ -11527,12 +11527,9 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener 
                } else {

                    widgetType = TextClassifier.WIDGET_TYPE_UNSELECTABLE_TEXTVIEW;

                }

                // TODO: Tagged this widgetType with a * so it we can monitor if it reports

                // SelectionEvents exactly as the older Logger does. Remove once investigations

                // are complete.

                final TextClassificationContext textClassificationContext =

                        new TextClassificationContext.Builder(

                                mContext.getPackageName(), "*" + widgetType)

                                mContext.getPackageName(), widgetType)

                                .build();

                if (mTextClassifier != null) {

                    mTextClassificationSession = tcm.createTextClassificationSession(

core/tests/coretests/src/android/widget/TextViewActivityTest.java

+29 −0
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ import static android.widget.espresso.TextViewAssertions.doesNotHaveStyledText; 
import static android.widget.espresso.TextViewAssertions.hasInsertionPointerAtIndex;

import static android.widget.espresso.TextViewAssertions.hasSelection;



import static junit.framework.Assert.assertEquals;

import static junit.framework.Assert.assertFalse;

import static junit.framework.Assert.assertTrue;
@@ -77,6 +78,7 @@ import android.view.ActionMode; 
import android.view.KeyEvent;

import android.view.Menu;

import android.view.MenuItem;

import android.view.textclassifier.SelectionEvent;

import android.view.textclassifier.TextClassificationManager;

import android.view.textclassifier.TextClassifier;

import android.view.textclassifier.TextLinks;
@@ -90,6 +92,9 @@ import org.junit.Rule; 
import org.junit.Test;

import org.junit.runner.RunWith;



import java.util.ArrayList;

import java.util.List;



/**

 * Tests the TextView widget from an Activity

 */
@@ -977,6 +982,30 @@ public class TextViewActivityTest { 
        assertFloatingToolbarDoesNotContainItem(android.R.id.textAssist);

    }



    @Test

    public void testSelectionMetricsLogger_noAbandonAfterCopy() throws Throwable {

        final List<SelectionEvent> selectionEvents = new ArrayList<>();

        final TextClassifier classifier = new TextClassifier() {

            @Override

            public void onSelectionEvent(SelectionEvent event) {

                selectionEvents.add(event);

            }

        };

        final TextView textView = mActivity.findViewById(R.id.textview);

        mActivityRule.runOnUiThread(() -> textView.setTextClassifier(classifier));

        mInstrumentation.waitForIdleSync();

        final String text = "andyroid@android.com";



        onView(withId(R.id.textview)).perform(replaceText(text));

        onView(withId(R.id.textview)).perform(longPressOnTextAtIndex(text.indexOf('@')));

        sleepForFloatingToolbarPopup();

        clickFloatingToolbarItem(mActivity.getString(com.android.internal.R.string.copy));

        mInstrumentation.waitForIdleSync();



        final SelectionEvent lastEvent = selectionEvents.get(selectionEvents.size() - 1);

        assertEquals(SelectionEvent.ACTION_COPY, lastEvent.getEventType());

    }



    @Test

    public void testPastePlainText_menuAction() {

        initializeClipboardWithText(TextStyle.STYLED);