1123514 - crash in java.lang.IndexOutOfBoundsException: replace (1261 ... 1260) has end before start at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Keyboards and IME, defect)

Description

[David Weir (satdav)]

This bug was filed from the Socorro interface and is 
report bp-5634f951-6ccb-4af9-bd68-a22d02150120.
=============================================================

This crashed on etherpad today

Comment 1

[Aaron Train [:aaronmt]]

java.lang.IndexOutOfBoundsException: replace (1261 ... 1260) has end before start
	at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java:1009)
	at android.text.SpannableStringBuilder.replace(SpannableStringBuilder.java:441)
	at android.text.SpannableStringBuilder.delete(SpannableStringBuilder.java:212)
	at org.mozilla.gecko.GeckoEditable.geckoReplaceText(GeckoEditable.java:861)
	at org.mozilla.gecko.GeckoEditable.onTextChange(GeckoEditable.java:942)
	at org.mozilla.gecko.GeckoAppShell.notifyIMEChange(GeckoAppShell.java:488)
	at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
	at org.mozilla.gecko.mozglue.GeckoLoader.nativeRun(Native Method)
	at org.mozilla.gecko.GeckoAppShell.runGecko(GeckoAppShell.java:325)
	at org.mozilla.gecko.GeckoThread.run(GeckoThread.java:184)

Can you provide steps to reproduce? Do you crash on Nightly https://ftp.mozilla.org/pub/mozilla.org/mobile/nightly/latest-mozilla-central-android-api-11/fennec-38.0a1.multi.android-arm.apk?

Comment 2

[David Weir (satdav)]

Was looking through websites then went to 

https://contributors.etherpad.mozilla.org/smjan2015

Once I was there started typing and it crasher

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Add test for wrong offset calculation (v1)

Add a test that creates conditions for wrong offset calculation and
potentially causes an exception.

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Correctly calculate text replacement offset (v1)

Fix a mistake in calculating the correct offset for one of the replacement
steps, which caused the IndexOutOfBoundsException. The old code used `oldEnd`
for the second text replacement without taking into account the offset change
as a result of the first text replacement that was already performed. The new
code correctly takes the offset, `delta`, into account.

Comment 6

[Eugen Sawin [:esawin]]

Comment on attachment 8807300 [details] [diff] [review]
Correctly calculate text replacement offset (v1)

Review of attachment 8807300 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditable.java
@@ +1229,5 @@
>                  // replacement in parts. First replace part of text before the sequence.
>                  mText.currentReplace(start, action.mStart, text.subSequence(0, indexInText));
>  
>                  // Then replace part of the text after the sequence.
> +                final int delta = indexInText + start - action.mStart;

I think it's clearer to leave actionStart and additionally add delta instead of replacing it.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/803d5895353d
Add test for wrong offset calculation; r=esawin
https://hg.mozilla.org/integration/mozilla-inbound/rev/7fa6baac8f37
Correctly calculate text replacement offset; r=esawin

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/803d5895353d
https://hg.mozilla.org/mozilla-central/rev/7fa6baac8f37

Comment 9

[Chris Peterson [:cpeterson]]

Removing steps-wanted keyword because this bug has been resolved.