From bc6862300fe5675addfe4ed5d0c7c345aad463aa Mon Sep 17 00:00:00 2001 From: Gilles Debunne Date: Fri, 6 Apr 2012 14:37:46 -0700 Subject: [PATCH] Revert "Faster and simpler replace in SSB" Bug 6300658 This change reveals a weird race condition where sometimes the text is entered twice. Adding a debugger slows down everything, and the problem is no longer reproducable. Reverting for now. This reverts commit ebd9a23817052c4d2aaa1058efa2b80b08003d4a. --- .../android/text/SpannableStringBuilder.java | 67 +++++++++++++------ 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/core/java/android/text/SpannableStringBuilder.java b/core/java/android/text/SpannableStringBuilder.java index bb4b282d7664c..f7a7eb8f65f48 100644 --- a/core/java/android/text/SpannableStringBuilder.java +++ b/core/java/android/text/SpannableStringBuilder.java @@ -50,8 +50,6 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable public SpannableStringBuilder(CharSequence text, int start, int end) { int srclen = end - start; - if (srclen < 0) throw new StringIndexOutOfBoundsException(); - int len = ArrayUtils.idealCharArraySize(srclen + 1); mText = new char[len]; mGapStart = srclen; @@ -155,7 +153,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable if (where == mGapStart) return; - boolean atEnd = (where == length()); + boolean atend = (where == length()); if (where < mGapStart) { int overlap = mGapStart - where; @@ -181,7 +179,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable else if (start == where) { int flag = (mSpanFlags[i] & START_MASK) >> START_SHIFT; - if (flag == POINT || (atEnd && flag == PARAGRAPH)) + if (flag == POINT || (atend && flag == PARAGRAPH)) start += mGapLength; } @@ -192,7 +190,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable else if (end == where) { int flag = (mSpanFlags[i] & END_MASK); - if (flag == POINT || (atEnd && flag == PARAGRAPH)) + if (flag == POINT || (atend && flag == PARAGRAPH)) end += mGapLength; } @@ -399,7 +397,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable // Documentation from interface public SpannableStringBuilder replace(final int start, final int end, - CharSequence tb, int tbstart, int tbend) { + CharSequence tb, int tbstart, int tbend) { int filtercount = mFilters.length; for (int i = 0; i < filtercount; i++) { CharSequence repl = mFilters[i].filter(tb, tbstart, tbend, this, start, end); @@ -421,26 +419,53 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable TextWatcher[] textWatchers = getSpans(start, start + origLen, TextWatcher.class); sendBeforeTextChanged(textWatchers, start, origLen, newLen); - // Try to keep the cursor / selection at the same relative position during - // a text replacement. If replaced or replacement text length is zero, this - // is already taken care of. - boolean adjustSelection = origLen != 0 && newLen != 0; - int selstart = 0; - int selend = 0; - if (adjustSelection) { - selstart = Selection.getSelectionStart(this); - selend = Selection.getSelectionEnd(this); - } + if (origLen == 0 || newLen == 0) { + change(start, end, tb, tbstart, tbend); + } else { + int selstart = Selection.getSelectionStart(this); + int selend = Selection.getSelectionEnd(this); - checkRange("replace", start, end); + // XXX just make the span fixups in change() do the right thing + // instead of this madness! - change(start, end, tb, tbstart, tbend); + checkRange("replace", start, end); + moveGapTo(end); - if (adjustSelection) { + if (mGapLength < 2) + resizeFor(length() + 1); + + for (int i = mSpanCount - 1; i >= 0; i--) { + if (mSpanStarts[i] == mGapStart) + mSpanStarts[i]++; + + if (mSpanEnds[i] == mGapStart) + mSpanEnds[i]++; + } + + mText[mGapStart] = ' '; + mGapStart++; + mGapLength--; + + if (mGapLength < 1) { + new Exception("mGapLength < 1").printStackTrace(); + } + + change(start + 1, start + 1, tb, tbstart, tbend); + change(start, start + 1, "", 0, 0); + change(start + newLen, start + newLen + origLen, "", 0, 0); + + /* + * Special case to keep the cursor in the same position + * if it was somewhere in the middle of the replaced region. + * If it was at the start or the end or crossing the whole + * replacement, it should already be where it belongs. + * TODO: Is there some more general mechanism that could + * accomplish this? + */ if (selstart > start && selstart < end) { long off = selstart - start; - off = off * newLen / origLen; + off = off * newLen / (end - start); selstart = (int) off + start; setSpan(false, Selection.SELECTION_START, selstart, selstart, @@ -449,7 +474,7 @@ public class SpannableStringBuilder implements CharSequence, GetChars, Spannable if (selend > start && selend < end) { long off = selend - start; - off = off * newLen / origLen; + off = off * newLen / (end - start); selend = (int) off + start; setSpan(false, Selection.SELECTION_END, selend, selend, Spanned.SPAN_POINT_POINT);