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

Commit d2026686 authored by James Cook's avatar James Cook
Browse files

Improve undo support for text entered with IME

Use span properties to detect:
* Composing text - don't record undo operations
* Completing a composition - record an insert undo operation
* Canceling a composition - don't record

Save the composition state on parcel/unparcel.

Stop using begin/end batch edit to try to detect when a TextWatcher
is modifying the text. IMEs trigger multiple InputFilter passes in
a single batch edit. Use SpannableStringBuilder to determine when
we're in a TextWatcher callback because it is the authority on that
state.

Fix a bug in undo manager where it doesn't forget undos correctly if
there are more than one in the stack.

Bug: 19332904
Change-Id: Iaa9b0b2a7bf6683302cc85e7616e5d5fcc9fa202
parent 11b07c05
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -30749,6 +30749,7 @@ package android.text {
    method public int getSpanStart(java.lang.Object);
    method public T[] getSpans(int, int, java.lang.Class<T>);
    method public deprecated int getTextRunCursor(int, int, int, int, int, android.graphics.Paint);
    method public int getTextWatcherDepth();
    method public android.text.SpannableStringBuilder insert(int, java.lang.CharSequence, int, int);
    method public android.text.SpannableStringBuilder insert(int, java.lang.CharSequence);
    method public int length();
+1 −0
Original line number Diff line number Diff line
@@ -33105,6 +33105,7 @@ package android.text {
    method public int getSpanStart(java.lang.Object);
    method public T[] getSpans(int, int, java.lang.Class<T>);
    method public deprecated int getTextRunCursor(int, int, int, int, int, android.graphics.Paint);
    method public int getTextWatcherDepth();
    method public android.text.SpannableStringBuilder insert(int, java.lang.CharSequence, int, int);
    method public android.text.SpannableStringBuilder insert(int, java.lang.CharSequence);
    method public int length();
+15 −14
Original line number Diff line number Diff line
@@ -119,15 +119,13 @@ public class UndoManager {
    }

    /**
     * Flatten the current undo state into a Parcelable object, which can later be restored
     * with {@link #restoreInstanceState(android.os.Parcelable)}.
     * Flatten the current undo state into a Parcel object, which can later be restored
     * with {@link #restoreInstanceState(android.os.Parcel, java.lang.ClassLoader)}.
     */
    public Parcelable saveInstanceState() {
    public void saveInstanceState(Parcel p) {
        if (mUpdateCount > 0) {
            throw new IllegalStateException("Can't save state while updating");
        }
        ParcelableParcel pp = new ParcelableParcel(getClass().getClassLoader());
        Parcel p = pp.getParcel();
        mStateSeq++;
        if (mStateSeq <= 0) {
            mStateSeq = 0;
@@ -151,7 +149,6 @@ public class UndoManager {
            mRedos.get(i).writeToParcel(p);
        }
        p.writeInt(0);
        return pp;
    }

    void saveOwner(UndoOwner owner, Parcel out) {
@@ -168,26 +165,24 @@ public class UndoManager {
    }

    /**
     * Restore an undo state previously created with {@link #saveInstanceState()}.  This will
     * restore the UndoManager's state to almost exactly what it was at the point it had
     * Restore an undo state previously created with {@link #saveInstanceState(Parcel)}.  This
     * will restore the UndoManager's state to almost exactly what it was at the point it had
     * been previously saved; the only information not restored is the data object
     * associated with each {@link UndoOwner}, which requires separate calls to
     * {@link #getOwner(String, Object)} to re-associate the owner with its data.
     */
    public void restoreInstanceState(Parcelable state) {
    public void restoreInstanceState(Parcel p, ClassLoader loader) {
        if (mUpdateCount > 0) {
            throw new IllegalStateException("Can't save state while updating");
        }
        forgetUndos(null, -1);
        forgetRedos(null, -1);
        ParcelableParcel pp = (ParcelableParcel)state;
        Parcel p = pp.getParcel();
        mHistorySize = p.readInt();
        mStateOwners = new UndoOwner[p.readInt()];

        int stype;
        while ((stype=p.readInt()) != 0) {
            UndoState ustate = new UndoState(this, p, pp.getClassLoader());
            UndoState ustate = new UndoState(this, p, loader);
            if (stype == 1) {
                mUndos.add(0, ustate);
            } else {
@@ -311,12 +306,15 @@ public class UndoManager {
        }

        int removed = 0;
        for (int i=0; i<mUndos.size() && removed < count; i++) {
        int i = 0;
        while (i < mUndos.size() && removed < count) {
            UndoState state = mUndos.get(i);
            if (count > 0 && matchOwners(state, owners)) {
                state.destroy();
                mUndos.remove(i);
                removed++;
            } else {
                i++;
            }
        }

@@ -329,12 +327,15 @@ public class UndoManager {
        }

        int removed = 0;
        for (int i=0; i<mRedos.size() && removed < count; i++) {
        int i = 0;
        while (i < mRedos.size() && removed < count) {
            UndoState state = mRedos.get(i);
            if (count > 0 && matchOwners(state, owners)) {
                state.destroy();
                mRedos.remove(i);
                removed++;
            } else {
                i++;
            }
        }

+19 −0
Original line number Diff line number Diff line
@@ -1006,28 +1006,43 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable
        return new String(buf);
    }

    /**
     * Returns the depth of TextWatcher callbacks. Returns 0 when the object is not handling
     * TextWatchers. A return value greater than 1 implies that a TextWatcher caused a change that
     * recursively triggered a TextWatcher.
     */
    public int getTextWatcherDepth() {
        return mTextWatcherDepth;
    }

    private void sendBeforeTextChanged(TextWatcher[] watchers, int start, int before, int after) {
        int n = watchers.length;

        mTextWatcherDepth++;
        for (int i = 0; i < n; i++) {
            watchers[i].beforeTextChanged(this, start, before, after);
        }
        mTextWatcherDepth--;
    }

    private void sendTextChanged(TextWatcher[] watchers, int start, int before, int after) {
        int n = watchers.length;

        mTextWatcherDepth++;
        for (int i = 0; i < n; i++) {
            watchers[i].onTextChanged(this, start, before, after);
        }
        mTextWatcherDepth--;
    }

    private void sendAfterTextChanged(TextWatcher[] watchers) {
        int n = watchers.length;

        mTextWatcherDepth++;
        for (int i = 0; i < n; i++) {
            watchers[i].afterTextChanged(this);
        }
        mTextWatcherDepth--;
    }

    private void sendSpanAdded(Object what, int start, int end) {
@@ -1524,6 +1539,10 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable
    private IdentityHashMap<Object, Integer> mIndexOfSpan;
    private int mLowWaterMark;  // indices below this have not been touched

    // TextWatcher callbacks may trigger changes that trigger more callbacks. This keeps track of
    // how deep the callbacks go.
    private int mTextWatcherDepth;

    // TODO These value are tightly related to the public SPAN_MARK/POINT values in {@link Spanned}
    private static final int MARK = 1;
    private static final int POINT = 2;
+96 −20
Original line number Diff line number Diff line
@@ -236,12 +236,17 @@ public class Editor {
    }

    ParcelableParcel saveInstanceState() {
        // For now there is only undo state.
        return (ParcelableParcel) mUndoManager.saveInstanceState();
        ParcelableParcel state = new ParcelableParcel(getClass().getClassLoader());
        Parcel parcel = state.getParcel();
        mUndoManager.saveInstanceState(parcel);
        mUndoInputFilter.saveInstanceState(parcel);
        return state;
    }

    void restoreInstanceState(ParcelableParcel state) {
        mUndoManager.restoreInstanceState(state);
        Parcel parcel = state.getParcel();
        mUndoManager.restoreInstanceState(parcel, state.getClassLoader());
        mUndoInputFilter.restoreInstanceState(parcel);
        // Re-associate this object as the owner of undo state.
        mUndoOwner = mUndoManager.getOwner(UNDO_OWNER_TAG, this);
    }
@@ -4576,20 +4581,30 @@ public class Editor {
        // Whether the current filter pass is directly caused by an end-user text edit.
        private boolean mIsUserEdit;

        // Whether this is the first pass through the filter for a given end-user text edit.
        private boolean mFirstFilterPass;
        // Whether the text field is handling an IME composition. Must be parceled in case the user
        // rotates the screen during composition.
        private boolean mHasComposition;

        public UndoInputFilter(Editor editor) {
            mEditor = editor;
        }

        public void saveInstanceState(Parcel parcel) {
            parcel.writeInt(mIsUserEdit ? 1 : 0);
            parcel.writeInt(mHasComposition ? 1 : 0);
        }

        public void restoreInstanceState(Parcel parcel) {
            mIsUserEdit = parcel.readInt() != 0;
            mHasComposition = parcel.readInt() != 0;
        }

        /**
         * Signals that a user-triggered edit is starting.
         */
        public void beginBatchEdit() {
            if (DEBUG_UNDO) Log.d(TAG, "beginBatchEdit");
            mIsUserEdit = true;
            mFirstFilterPass = true;
        }

        public void endBatchEdit() {
@@ -4610,17 +4625,63 @@ public class Editor {
                return null;
            }

            // Check for and handle IME composition edits.
            if (handleCompositionEdit(source, start, end, dstart)) {
                return null;
            }

            // Handle keyboard edits.
            handleKeyboardEdit(source, start, end, dest, dstart, dend);
            return null;
        }

        /**
         * Returns true iff the edit was handled, either because it should be ignored or because
         * this function created an undo operation for it.
         */
        private boolean handleCompositionEdit(CharSequence source, int start, int end, int dstart) {
            // Ignore edits while the user is composing.
            if (isComposition(source)) {
                mHasComposition = true;
                return true;
            }
            final boolean hadComposition = mHasComposition;
            mHasComposition = false;

            // Check for the transition out of the composing state.
            if (hadComposition) {
                // If there was no text the user canceled composition. Ignore the edit.
                if (start == end) {
                    return true;
                }

                // Otherwise the user inserted the composition.
                String newText = TextUtils.substring(source, start, end);
                EditOperation edit = new EditOperation(mEditor, false, "", dstart, newText);
                recordEdit(edit);
                return true;
            }

            // This was neither a composition event nor a transition out of composing.
            return false;
        }

        private void handleKeyboardEdit(CharSequence source, int start, int end,
                Spanned dest, int dstart, int dend) {
            // An application may install a TextWatcher to provide additional modifications after
            // the initial input filters run (e.g. a credit card formatter that adds spaces to a
            // string). This results in multiple filter() calls for what the user considers to be
            // a single operation. Always undo the whole set of changes in one step.
            final boolean forceMerge = !mFirstFilterPass;
            mFirstFilterPass = false;
            final boolean forceMerge = isInTextWatcher();

            // Build a new operation with all the information from this edit.
            EditOperation edit = new EditOperation(mEditor, forceMerge,
                    source, start, end, dest, dstart, dend);
            String newText = TextUtils.substring(source, start, end);
            String oldText = TextUtils.substring(dest, dstart, dend);
            EditOperation edit = new EditOperation(mEditor, forceMerge, oldText, dstart, newText);
            recordEdit(edit);
        }

        private void recordEdit(EditOperation edit) {
            // Fetch the last edit operation and attempt to merge in the new edit.
            final UndoManager um = mEditor.mUndoManager;
            um.beginUpdate("Edit text");
@@ -4646,7 +4707,6 @@ public class Editor {
                um.addOperation(edit, UndoManager.MERGE_MODE_NONE);
            }
            um.endUpdate();
            return null;  // Text not changed.
        }

        private boolean canUndoEdit(CharSequence source, int start, int end,
@@ -4678,6 +4738,23 @@ public class Editor {

            return true;
        }

        private boolean isComposition(CharSequence source) {
            if (!(source instanceof Spannable)) {
                return false;
            }
            // This is a composition edit if the source has a non-zero-length composing span.
            Spannable text = (Spannable) source;
            int composeBegin = EditableInputConnection.getComposingSpanStart(text);
            int composeEnd = EditableInputConnection.getComposingSpanEnd(text);
            return composeBegin < composeEnd;
        }

        private boolean isInTextWatcher() {
            CharSequence text = mEditor.mTextView.getText();
            return (text instanceof SpannableStringBuilder)
                    && ((SpannableStringBuilder) text).getTextWatcherDepth() > 0;
        }
    }

    /**
@@ -4699,17 +4776,16 @@ public class Editor {
        private int mNewCursorPos;

        /**
         * Constructs an edit operation from a text input operation that replaces the range
         * (dstart, dend) of dest with (start, end) of source. See {@link InputFilter#filter}.
         * If forceMerge is true then always forcibly merge this operation with any previous one.
         * Constructs an edit operation from a text input operation on editor that replaces the
         * oldText starting at dstart with newText. If forceMerge is true then always forcibly
         * merge this operation with any previous one.
         */
        public EditOperation(Editor editor, boolean forceMerge,
                CharSequence source, int start, int end, Spanned dest, int dstart, int dend) {
        public EditOperation(Editor editor, boolean forceMerge, String oldText, int dstart,
                String newText) {
            super(editor.mUndoOwner);
            mForceMerge = forceMerge;

            mOldText = dest.subSequence(dstart, dend).toString();
            mNewText = source.subSequence(start, end).toString();
            mOldText = oldText;
            mNewText = newText;

            // Determine the type of the edit and store where it occurred. Avoid storing
            // irrevelant data (e.g. mNewTextStart for a delete) because that makes the
@@ -4728,7 +4804,7 @@ public class Editor {

            // Store cursor data.
            mOldCursorPos = editor.mTextView.getSelectionStart();
            mNewCursorPos = dstart + (end - start);
            mNewCursorPos = dstart + mNewText.length();
        }

        public EditOperation(Parcel src, ClassLoader loader) {
Loading