Skip to content

Commit f6584a7

Browse files
Gilles DebunneAndroid (Google) Code Review
authored andcommitted
Merge "SpannableStringBuilder correctly broadcast span changes during replace"
2 parents 4a5af36 + 174c44c commit f6584a7

File tree

1 file changed

+144
-76
lines changed

1 file changed

+144
-76
lines changed

core/java/android/text/SpannableStringBuilder.java

Lines changed: 144 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public SpannableStringBuilder(CharSequence text, int start, int end) {
7575
if (spans[i] instanceof NoCopySpan) {
7676
continue;
7777
}
78-
78+
7979
int st = sp.getSpanStart(spans[i]) - start;
8080
int en = sp.getSpanEnd(spans[i]) - start;
8181
int fl = sp.getSpanFlags(spans[i]);
@@ -212,15 +212,15 @@ public SpannableStringBuilder delete(int start, int end) {
212212

213213
if (mGapLength > 2 * length())
214214
resizeFor(length());
215-
215+
216216
return ret; // == this
217217
}
218218

219219
// Documentation from interface
220220
public void clear() {
221221
replace(0, length(), "", 0, 0);
222222
}
223-
223+
224224
// Documentation from interface
225225
public void clearSpans() {
226226
for (int i = mSpanCount - 1; i >= 0; i--) {
@@ -257,45 +257,50 @@ public SpannableStringBuilder append(char text) {
257257
return append(String.valueOf(text));
258258
}
259259

260-
private void change(int start, int end, CharSequence tb, int tbstart, int tbend) {
261-
checkRange("replace", start, end);
260+
private void change(int start, int end, CharSequence cs, int csStart, int csEnd) {
261+
// Can be negative
262+
final int nbNewChars = (csEnd - csStart) - (end - start);
262263

263264
for (int i = mSpanCount - 1; i >= 0; i--) {
264-
if ((mSpanFlags[i] & SPAN_PARAGRAPH) == SPAN_PARAGRAPH) {
265-
int st = mSpanStarts[i];
266-
if (st > mGapStart)
267-
st -= mGapLength;
265+
int spanStart = mSpanStarts[i];
266+
if (spanStart > mGapStart)
267+
spanStart -= mGapLength;
268268

269-
int en = mSpanEnds[i];
270-
if (en > mGapStart)
271-
en -= mGapLength;
269+
int spanEnd = mSpanEnds[i];
270+
if (spanEnd > mGapStart)
271+
spanEnd -= mGapLength;
272272

273-
int ost = st;
274-
int oen = en;
273+
if ((mSpanFlags[i] & SPAN_PARAGRAPH) == SPAN_PARAGRAPH) {
274+
int ost = spanStart;
275+
int oen = spanEnd;
275276
int clen = length();
276277

277-
if (st > start && st <= end) {
278-
for (st = end; st < clen; st++)
279-
if (st > end && charAt(st - 1) == '\n')
278+
if (spanStart > start && spanStart <= end) {
279+
for (spanStart = end; spanStart < clen; spanStart++)
280+
if (spanStart > end && charAt(spanStart - 1) == '\n')
280281
break;
281282
}
282283

283-
if (en > start && en <= end) {
284-
for (en = end; en < clen; en++)
285-
if (en > end && charAt(en - 1) == '\n')
284+
if (spanEnd > start && spanEnd <= end) {
285+
for (spanEnd = end; spanEnd < clen; spanEnd++)
286+
if (spanEnd > end && charAt(spanEnd - 1) == '\n')
286287
break;
287288
}
288289

289-
if (st != ost || en != oen)
290-
setSpan(false, mSpans[i], st, en, mSpanFlags[i]);
290+
if (spanStart != ost || spanEnd != oen)
291+
setSpan(false, mSpans[i], spanStart, spanEnd, mSpanFlags[i]);
291292
}
293+
294+
int flags = 0;
295+
if (spanStart == start) flags |= SPAN_START_AT_START;
296+
else if (spanStart == end + nbNewChars) flags |= SPAN_START_AT_END;
297+
if (spanEnd == start) flags |= SPAN_END_AT_START;
298+
else if (spanEnd == end + nbNewChars) flags |= SPAN_END_AT_END;
299+
mSpanFlags[i] |= flags;
292300
}
293301

294302
moveGapTo(end);
295303

296-
// Can be negative
297-
final int nbNewChars = (tbend - tbstart) - (end - start);
298-
299304
if (nbNewChars >= mGapLength) {
300305
resizeFor(mText.length + nbNewChars - mGapLength);
301306
}
@@ -306,7 +311,7 @@ private void change(int start, int end, CharSequence tb, int tbstart, int tbend)
306311
if (mGapLength < 1)
307312
new Exception("mGapLength < 1").printStackTrace();
308313

309-
TextUtils.getChars(tb, tbstart, tbend, mText, start);
314+
TextUtils.getChars(cs, csStart, csEnd, mText, start);
310315

311316
if (end > start) {
312317
// no need for span fixup on pure insertion
@@ -340,21 +345,23 @@ private void change(int start, int end, CharSequence tb, int tbstart, int tbend)
340345
}
341346
}
342347

343-
if (tb instanceof Spanned) {
344-
Spanned sp = (Spanned) tb;
345-
Object[] spans = sp.getSpans(tbstart, tbend, Object.class);
348+
mSpanCountBeforeAdd = mSpanCount;
349+
350+
if (cs instanceof Spanned) {
351+
Spanned sp = (Spanned) cs;
352+
Object[] spans = sp.getSpans(csStart, csEnd, Object.class);
346353

347354
for (int i = 0; i < spans.length; i++) {
348355
int st = sp.getSpanStart(spans[i]);
349356
int en = sp.getSpanEnd(spans[i]);
350357

351-
if (st < tbstart) st = tbstart;
352-
if (en > tbend) en = tbend;
358+
if (st < csStart) st = csStart;
359+
if (en > csEnd) en = csEnd;
353360

354361
// Add span only if this object is not yet used as a span in this string
355-
if (getSpanStart(spans[i]) < 0) {
356-
setSpan(false, spans[i], st - tbstart + start, en - tbstart + start,
357-
sp.getSpanFlags(spans[i]));
362+
if (getSpanStart(spans[i]) < 0 && !(spans[i] instanceof SpanWatcher)) {
363+
setSpan(false, spans[i], st - csStart + start, en - csStart + start,
364+
sp.getSpanFlags(spans[i]));
358365
}
359366
}
360367
}
@@ -390,6 +397,8 @@ public SpannableStringBuilder replace(int start, int end, CharSequence tb) {
390397
// Documentation from interface
391398
public SpannableStringBuilder replace(final int start, final int end,
392399
CharSequence tb, int tbstart, int tbend) {
400+
checkRange("replace", start, end);
401+
393402
int filtercount = mFilters.length;
394403
for (int i = 0; i < filtercount; i++) {
395404
CharSequence repl = mFilters[i].filter(tb, tbstart, tbend, this, start, end);
@@ -404,54 +413,108 @@ public SpannableStringBuilder replace(final int start, final int end,
404413
final int origLen = end - start;
405414
final int newLen = tbend - tbstart;
406415

407-
if (origLen == 0 && newLen == 0) {
408-
return this;
409-
}
410-
411416
TextWatcher[] textWatchers = getSpans(start, start + origLen, TextWatcher.class);
412417
sendBeforeTextChanged(textWatchers, start, origLen, newLen);
413418

414419
// Try to keep the cursor / selection at the same relative position during
415420
// a text replacement. If replaced or replacement text length is zero, this
416421
// is already taken care of.
417422
boolean adjustSelection = origLen != 0 && newLen != 0;
418-
int selstart = 0;
419-
int selend = 0;
423+
int selectionStart = 0;
424+
int selectionEnd = 0;
420425
if (adjustSelection) {
421-
selstart = Selection.getSelectionStart(this);
422-
selend = Selection.getSelectionEnd(this);
426+
selectionStart = Selection.getSelectionStart(this);
427+
selectionEnd = Selection.getSelectionEnd(this);
423428
}
424429

425-
checkRange("replace", start, end);
426-
427430
change(start, end, tb, tbstart, tbend);
428431

429432
if (adjustSelection) {
430-
if (selstart > start && selstart < end) {
431-
long off = selstart - start;
432-
433-
off = off * newLen / origLen;
434-
selstart = (int) off + start;
433+
if (selectionStart > start && selectionStart < end) {
434+
final int offset = (selectionStart - start) * newLen / origLen;
435+
selectionStart = start + offset;
435436

436-
setSpan(false, Selection.SELECTION_START, selstart, selstart,
437+
setSpan(false, Selection.SELECTION_START, selectionStart, selectionStart,
437438
Spanned.SPAN_POINT_POINT);
438439
}
439-
if (selend > start && selend < end) {
440-
long off = selend - start;
440+
if (selectionEnd > start && selectionEnd < end) {
441+
final int offset = (selectionEnd - start) * newLen / origLen;
442+
selectionEnd = start + offset;
441443

442-
off = off * newLen / origLen;
443-
selend = (int) off + start;
444-
445-
setSpan(false, Selection.SELECTION_END, selend, selend, Spanned.SPAN_POINT_POINT);
444+
setSpan(false, Selection.SELECTION_END, selectionEnd, selectionEnd,
445+
Spanned.SPAN_POINT_POINT);
446446
}
447447
}
448448

449449
sendTextChanged(textWatchers, start, origLen, newLen);
450450
sendAfterTextChanged(textWatchers);
451451

452+
// Span watchers need to be called after text watchers, which may update the layout
453+
sendToSpanWatchers(start, end, newLen - origLen);
454+
452455
return this;
453456
}
454457

458+
private void sendToSpanWatchers(int replaceStart, int replaceEnd, int nbNewChars) {
459+
for (int i = 0; i < mSpanCountBeforeAdd; i++) {
460+
int spanStart = mSpanStarts[i];
461+
int spanEnd = mSpanEnds[i];
462+
if (spanStart > mGapStart) spanStart -= mGapLength;
463+
if (spanEnd > mGapStart) spanEnd -= mGapLength;
464+
int spanFlags = mSpanFlags[i];
465+
466+
int newReplaceEnd = replaceEnd + nbNewChars;
467+
boolean spanChanged = false;
468+
int previousSpanStart = spanStart;
469+
if (spanStart > newReplaceEnd) {
470+
if (nbNewChars != 0) {
471+
previousSpanStart -= nbNewChars;
472+
spanChanged = true;
473+
}
474+
} else if (spanStart >= replaceStart) {
475+
// No change if span start was already at replace interval boundaries before replace
476+
if ((spanStart != replaceStart ||
477+
((spanFlags & SPAN_START_AT_START) != SPAN_START_AT_START)) &&
478+
(spanStart != newReplaceEnd ||
479+
((spanFlags & SPAN_START_AT_END) != SPAN_START_AT_END))) {
480+
// TODO previousSpanStart is incorrect, but we would need to save all the
481+
// previous spans' positions before replace to provide it
482+
spanChanged = true;
483+
}
484+
}
485+
int previousSpanEnd = spanEnd;
486+
if (spanEnd > newReplaceEnd) {
487+
if (nbNewChars != 0) {
488+
previousSpanEnd -= nbNewChars;
489+
spanChanged = true;
490+
}
491+
} else if (spanEnd >= replaceStart) {
492+
// No change if span start was already at replace interval boundaries before replace
493+
if ((spanEnd != replaceStart ||
494+
((spanFlags & SPAN_END_AT_START) != SPAN_END_AT_START)) &&
495+
(spanEnd != newReplaceEnd ||
496+
((spanFlags & SPAN_END_AT_END) != SPAN_END_AT_END))) {
497+
// TODO same as above for previousSpanEnd
498+
spanChanged = true;
499+
}
500+
}
501+
502+
if (spanChanged) {
503+
sendSpanChanged(mSpans[i], previousSpanStart, previousSpanEnd, spanStart, spanEnd);
504+
}
505+
mSpanFlags[i] &= ~SPAN_START_END_MASK;
506+
}
507+
508+
// The spans starting at mIntermediateSpanCount were added from the replacement text
509+
for (int i = mSpanCountBeforeAdd; i < mSpanCount; i++) {
510+
int spanStart = mSpanStarts[i];
511+
int spanEnd = mSpanEnds[i];
512+
if (spanStart > mGapStart) spanStart -= mGapLength;
513+
if (spanEnd > mGapStart) spanEnd -= mGapLength;
514+
sendSpanAdded(mSpans[i], spanStart, spanEnd);
515+
}
516+
}
517+
455518
/**
456519
* Mark the specified range of text with the specified object.
457520
* The flags determine how the span will behave when text is
@@ -788,13 +851,12 @@ public void getChars(int start, int end, char[] dest, int destoff) {
788851
if (end <= mGapStart) {
789852
System.arraycopy(mText, start, dest, destoff, end - start);
790853
} else if (start >= mGapStart) {
791-
System.arraycopy(mText, start + mGapLength,
792-
dest, destoff, end - start);
854+
System.arraycopy(mText, start + mGapLength, dest, destoff, end - start);
793855
} else {
794856
System.arraycopy(mText, start, dest, destoff, mGapStart - start);
795857
System.arraycopy(mText, mGapStart + mGapLength,
796-
dest, destoff + (mGapStart - start),
797-
end - mGapStart);
858+
dest, destoff + (mGapStart - start),
859+
end - mGapStart);
798860
}
799861
}
800862

@@ -863,12 +925,14 @@ private void sendSpanRemoved(Object what, int start, int end) {
863925
}
864926
}
865927

866-
private void sendSpanChanged(Object what, int s, int e, int st, int en) {
867-
SpanWatcher[] recip = getSpans(Math.min(s, st), Math.max(e, en), SpanWatcher.class);
868-
int n = recip.length;
869-
928+
private void sendSpanChanged(Object what, int oldStart, int oldEnd, int start, int end) {
929+
// The bounds of a possible SpanWatcher are guaranteed to be set before this method is
930+
// called, so that the order of the span does not affect this broadcast.
931+
SpanWatcher[] spanWatchers = getSpans(Math.min(oldStart, start),
932+
Math.min(Math.max(oldEnd, end), length()), SpanWatcher.class);
933+
int n = spanWatchers.length;
870934
for (int i = 0; i < n; i++) {
871-
recip[i].onSpanChanged(this, what, s, e, st, en);
935+
spanWatchers[i].onSpanChanged(this, what, oldStart, oldEnd, start, end);
872936
}
873937
}
874938

@@ -879,26 +943,23 @@ private static String region(int start, int end) {
879943
private void checkRange(final String operation, int start, int end) {
880944
if (end < start) {
881945
throw new IndexOutOfBoundsException(operation + " " +
882-
region(start, end) +
883-
" has end before start");
946+
region(start, end) + " has end before start");
884947
}
885948

886949
int len = length();
887950

888951
if (start > len || end > len) {
889952
throw new IndexOutOfBoundsException(operation + " " +
890-
region(start, end) +
891-
" ends beyond length " + len);
953+
region(start, end) + " ends beyond length " + len);
892954
}
893955

894956
if (start < 0 || end < 0) {
895957
throw new IndexOutOfBoundsException(operation + " " +
896-
region(start, end) +
897-
" starts before 0");
958+
region(start, end) + " starts before 0");
898959
}
899960
}
900961

901-
/*
962+
/*
902963
private boolean isprint(char c) { // XXX
903964
if (c >= ' ' && c <= '~')
904965
return true;
@@ -977,7 +1038,7 @@ public void dump() { // XXX
9771038
9781039
System.out.print("\n");
9791040
}
980-
*/
1041+
*/
9811042

9821043
/**
9831044
* Don't call this yourself -- exists for Canvas to use internally.
@@ -1023,7 +1084,7 @@ public void drawTextRun(Canvas c, int start, int end, int contextStart, int cont
10231084
}
10241085
}
10251086

1026-
/**
1087+
/**
10271088
* Don't call this yourself -- exists for Paint to use internally.
10281089
* {@hide}
10291090
*/
@@ -1059,8 +1120,7 @@ public int getTextWidths(int start, int end, float[] widths, Paint p) {
10591120
if (end <= mGapStart) {
10601121
ret = p.getTextWidths(mText, start, end - start, widths);
10611122
} else if (start >= mGapStart) {
1062-
ret = p.getTextWidths(mText, start + mGapLength, end - start,
1063-
widths);
1123+
ret = p.getTextWidths(mText, start + mGapLength, end - start, widths);
10641124
} else {
10651125
char[] buf = TextUtils.obtain(end - start);
10661126

@@ -1205,6 +1265,7 @@ public InputFilter[] getFilters() {
12051265
private int[] mSpanEnds;
12061266
private int[] mSpanFlags;
12071267
private int mSpanCount;
1268+
private int mSpanCountBeforeAdd;
12081269

12091270
// TODO These value are tightly related to the public SPAN_MARK/POINT values in {@link Spanned}
12101271
private static final int MARK = 1;
@@ -1214,4 +1275,11 @@ public InputFilter[] getFilters() {
12141275
private static final int START_MASK = 0xF0;
12151276
private static final int END_MASK = 0x0F;
12161277
private static final int START_SHIFT = 4;
1278+
1279+
// These bits are not (currently) used by SPANNED flags
1280+
private static final int SPAN_START_AT_START = 0x1000;
1281+
private static final int SPAN_START_AT_END = 0x2000;
1282+
private static final int SPAN_END_AT_START = 0x4000;
1283+
private static final int SPAN_END_AT_END = 0x8000;
1284+
private static final int SPAN_START_END_MASK = 0xF000;
12171285
}

0 commit comments

Comments
 (0)