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
diff --git a/api/current.txt b/api/current.txt
index 7bac613..b6a33c4 100644
--- a/api/current.txt
+++ b/api/current.txt
@@ -30749,6 +30749,7 @@
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();
diff --git a/api/system-current.txt b/api/system-current.txt
index 89c0460e..b154e3e 100644
--- a/api/system-current.txt
+++ b/api/system-current.txt
@@ -33105,6 +33105,7 @@
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();
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 @@
}
/**
- * 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 @@
mRedos.get(i).writeToParcel(p);
}
p.writeInt(0);
- return pp;
}
void saveOwner(UndoOwner owner, Parcel out) {
@@ -168,26 +165,24 @@
}
/**
- * 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 @@
}
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 @@
}
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 @@
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 @@
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 d93b212..c951c6e 100644
--- a/core/java/android/widget/Editor.java
+++ b/core/java/android/widget/Editor.java
@@ -236,12 +236,17 @@
}
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 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
// 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 447e9ac..4eb0841 100644
--- a/core/java/android/widget/TextView.java
+++ b/core/java/android/widget/TextView.java
@@ -111,8 +111,6 @@
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;
@@ -8959,9 +8957,10 @@
*
* 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
@@ -9536,7 +9535,6 @@
// 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;