Merge "Improve undo support for text entered with IME"

4 files changed, 134 insertions, 40 deletions

diff --git a/core/java/android/content/UndoManager.java b/core/java/android/content/UndoManager.java
index 1d5ed8a..46c2c6e 100644
--- a/core/java/android/content/UndoManager.java
+++ b/core/java/android/content/UndoManager.java
@@ -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++;
             }
         }
 
diff --git a/core/java/android/text/SpannableStringBuilder.java b/core/java/android/text/SpannableStringBuilder.java
index 7ce44e1..992dc4d 100644
--- a/core/java/android/text/SpannableStringBuilder.java
+++ b/core/java/android/text/SpannableStringBuilder.java
@@ -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;
diff --git a/core/java/android/widget/Editor.java b/core/java/android/widget/Editor.java
index 2cb6409..6d7a547 100644
--- a/core/java/android/widget/Editor.java
+++ b/core/java/android/widget/Editor.java
@@ -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);
     }
@@ -4590,20 +4595,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() {
@@ -4624,17 +4639,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");
@@ -4660,7 +4721,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,
@@ -4692,6 +4752,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;
+        }
     }
 
     /**
@@ -4713,17 +4790,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
@@ -4742,7 +4818,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) {
diff --git a/core/java/android/widget/TextView.java b/core/java/android/widget/TextView.java
index 2bfee1c..718ef93 100644
--- a/core/java/android/widget/TextView.java
+++ b/core/java/android/widget/TextView.java
@@ -111,8 +111,6 @@ import android.view.Gravity;
 import android.view.HapticFeedbackConstants;
 import android.view.KeyCharacterMap;
 import android.view.KeyEvent;
-import android.view.Menu;
-import android.view.MenuItem;
 import android.view.MotionEvent;
 import android.view.View;
 import android.view.ViewAssistStructure;
@@ -8957,9 +8955,10 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener
      *
      * A custom implementation can add new entries in the default menu in its
      * {@link android.view.ActionMode.Callback#onPrepareActionMode(ActionMode, Menu)} method. The
-     * default actions can also be removed from the menu using {@link Menu#removeItem(int)} and
-     * passing {@link android.R.id#selectAll}, {@link android.R.id#cut}, {@link android.R.id#copy}
-     * or {@link android.R.id#paste} ids as parameters.
+     * default actions can also be removed from the menu using
+     * {@link android.view.Menu#removeItem(int)} and passing {@link android.R.id#selectAll},
+     * {@link android.R.id#cut}, {@link android.R.id#copy} or {@link android.R.id#paste} ids as
+     * parameters.
      *
      * Returning false from
      * {@link android.view.ActionMode.Callback#onCreateActionMode(ActionMode, Menu)} will prevent
@@ -9534,7 +9533,6 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener
         // TODO: Add an option to configure this
         private static final float MARQUEE_DELTA_MAX = 0.07f;
         private static final int MARQUEE_DELAY = 1200;
-        private static final int MARQUEE_RESTART_DELAY = 1200;
         private static final int MARQUEE_DP_PER_SECOND = 30;
 
         private static final byte MARQUEE_STOPPED = 0x0;