Skip to content

Commit bc68623

Browse files
author
Gilles Debunne
committed
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 ebd9a23.
1 parent 5713c9c commit bc68623

File tree

1 file changed

+46
-21
lines changed

1 file changed

+46
-21
lines changed

core/java/android/text/SpannableStringBuilder.java

Lines changed: 46 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ public SpannableStringBuilder(CharSequence text) {
5050
public SpannableStringBuilder(CharSequence text, int start, int end) {
5151
int srclen = end - start;
5252

53-
if (srclen < 0) throw new StringIndexOutOfBoundsException();
54-
5553
int len = ArrayUtils.idealCharArraySize(srclen + 1);
5654
mText = new char[len];
5755
mGapStart = srclen;
@@ -155,7 +153,7 @@ private void moveGapTo(int where) {
155153
if (where == mGapStart)
156154
return;
157155

158-
boolean atEnd = (where == length());
156+
boolean atend = (where == length());
159157

160158
if (where < mGapStart) {
161159
int overlap = mGapStart - where;
@@ -181,7 +179,7 @@ private void moveGapTo(int where) {
181179
else if (start == where) {
182180
int flag = (mSpanFlags[i] & START_MASK) >> START_SHIFT;
183181

184-
if (flag == POINT || (atEnd && flag == PARAGRAPH))
182+
if (flag == POINT || (atend && flag == PARAGRAPH))
185183
start += mGapLength;
186184
}
187185

@@ -192,7 +190,7 @@ else if (start == where) {
192190
else if (end == where) {
193191
int flag = (mSpanFlags[i] & END_MASK);
194192

195-
if (flag == POINT || (atEnd && flag == PARAGRAPH))
193+
if (flag == POINT || (atend && flag == PARAGRAPH))
196194
end += mGapLength;
197195
}
198196

@@ -399,7 +397,7 @@ public SpannableStringBuilder replace(int start, int end, CharSequence tb) {
399397

400398
// Documentation from interface
401399
public SpannableStringBuilder replace(final int start, final int end,
402-
CharSequence tb, int tbstart, int tbend) {
400+
CharSequence tb, int tbstart, int tbend) {
403401
int filtercount = mFilters.length;
404402
for (int i = 0; i < filtercount; i++) {
405403
CharSequence repl = mFilters[i].filter(tb, tbstart, tbend, this, start, end);
@@ -421,26 +419,53 @@ public SpannableStringBuilder replace(final int start, final int end,
421419
TextWatcher[] textWatchers = getSpans(start, start + origLen, TextWatcher.class);
422420
sendBeforeTextChanged(textWatchers, start, origLen, newLen);
423421

424-
// Try to keep the cursor / selection at the same relative position during
425-
// a text replacement. If replaced or replacement text length is zero, this
426-
// is already taken care of.
427-
boolean adjustSelection = origLen != 0 && newLen != 0;
428-
int selstart = 0;
429-
int selend = 0;
430-
if (adjustSelection) {
431-
selstart = Selection.getSelectionStart(this);
432-
selend = Selection.getSelectionEnd(this);
433-
}
422+
if (origLen == 0 || newLen == 0) {
423+
change(start, end, tb, tbstart, tbend);
424+
} else {
425+
int selstart = Selection.getSelectionStart(this);
426+
int selend = Selection.getSelectionEnd(this);
434427

435-
checkRange("replace", start, end);
428+
// XXX just make the span fixups in change() do the right thing
429+
// instead of this madness!
430+
431+
checkRange("replace", start, end);
432+
moveGapTo(end);
433+
434+
if (mGapLength < 2)
435+
resizeFor(length() + 1);
436436

437-
change(start, end, tb, tbstart, tbend);
437+
for (int i = mSpanCount - 1; i >= 0; i--) {
438+
if (mSpanStarts[i] == mGapStart)
439+
mSpanStarts[i]++;
440+
441+
if (mSpanEnds[i] == mGapStart)
442+
mSpanEnds[i]++;
443+
}
444+
445+
mText[mGapStart] = ' ';
446+
mGapStart++;
447+
mGapLength--;
448+
449+
if (mGapLength < 1) {
450+
new Exception("mGapLength < 1").printStackTrace();
451+
}
438452

439-
if (adjustSelection) {
453+
change(start + 1, start + 1, tb, tbstart, tbend);
454+
change(start, start + 1, "", 0, 0);
455+
change(start + newLen, start + newLen + origLen, "", 0, 0);
456+
457+
/*
458+
* Special case to keep the cursor in the same position
459+
* if it was somewhere in the middle of the replaced region.
460+
* If it was at the start or the end or crossing the whole
461+
* replacement, it should already be where it belongs.
462+
* TODO: Is there some more general mechanism that could
463+
* accomplish this?
464+
*/
440465
if (selstart > start && selstart < end) {
441466
long off = selstart - start;
442467

443-
off = off * newLen / origLen;
468+
off = off * newLen / (end - start);
444469
selstart = (int) off + start;
445470

446471
setSpan(false, Selection.SELECTION_START, selstart, selstart,
@@ -449,7 +474,7 @@ public SpannableStringBuilder replace(final int start, final int end,
449474
if (selend > start && selend < end) {
450475
long off = selend - start;
451476

452-
off = off * newLen / origLen;
477+
off = off * newLen / (end - start);
453478
selend = (int) off + start;
454479

455480
setSpan(false, Selection.SELECTION_END, selend, selend, Spanned.SPAN_POINT_POINT);

0 commit comments

Comments
 (0)