Closed Bug 1127855 Opened 9 years ago Closed 6 years ago

crash in java.lang.IndexOutOfBoundsException: setSpan (N ... N) ends beyond length M at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)

Categories

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

35 Branch
All
Android
defect
Not set
critical

Tracking

(firefox62 wontfix, firefox63 fixed, firefox64 fixed)

RESOLVED FIXED
Firefox 64
Tracking Status
firefox62 --- wontfix
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: aaronmt, Assigned: gsvelto)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-6e5a6550-04d3-45ad-ab2f-b42c72150129.
=============================================================

java.lang.IndexOutOfBoundsException: setSpan (424 ... 424) ends beyond length 423
	at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java:945)
	at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:527)
	at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:520)
	at org.mozilla.gecko.GeckoEditable.notifyIME(GeckoEditable.java:741)
	at org.mozilla.gecko.GeckoAppShell.notifyIME(GeckoAppShell.java:470)
	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)

Saw this reported on Twitter https://twitter.com/creatrixtiara/status/560721309672685568
Age old issue?

Bug 788482 same crash function but an old stack.
Keywords: steps-wanted
Summary: crash in java.lang.IndexOutOfBoundsException: setSpan (N ... N) ends beyond length N at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java) → crash in java.lang.IndexOutOfBoundsException: setSpan (N ... N) ends beyond length M at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)
Crash Signature: [@ java.lang.IndexOutOfBoundsException: setSpan (424 ... 424) ends beyond length 423 at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)] → [@ java.lang.IndexOutOfBoundsException: setSpan (424 ... 424) ends beyond length 423 at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)] [@ java.lang.IndexOutOfBoundsException: setSpan ends beyond length 423 at android.text.Sp…
Crash Signature: android.text.SpannableStringBuilder.checkRange] → android.text.SpannableStringBuilder.checkRange] [@ java.lang.IndexOutOfBoundsException: setSpan (44 ... 45) ends beyond length 41 at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)] [@ java.lang.IndexOutOfBoundsException: se…
Most of these have the end index passed to setSpan one past the length of the string, but there are a few cases where the start index is past the end of the string, too, e.g. in "setSpan (51 ... 52) ends beyond length 49". Given that the index comes from an "indexOf" call performed just previously (https://dxr.mozilla.org/mozilla-central/rev/71a2dacbbb8ba2f3522432ff7e8fc0151ca591f9/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#1228), I'm not sure what to make of that. The only suspicious thing is that we're doing our search on copies of the string that have been lower cased, but then set the span on the original string. Whether that is actually responsible for the index mismatches we're seeing I've no idea, though.
In my case ( https://bugzilla.mozilla.org/show_bug.cgi?id=1406739 ) typing t24.com.tr crashes the browser right when you tap '4' key. So I tried typing t25,t26,ta it doesn't happen. Almost like t24 hits a magic value.
I think I have STR for one variant of this bug. You need to be running on a tablet to get the tablet GUI that allows you to switch tabs whilst typing into the address bar.

1. Start Firefox for Android
2. Open another tab so you have a total of two tabs open
3. Manually type (i.e. not paste) about:home into the address bar. Do not press return.
4. Switch to the other tab
5. Switch back to the original tab

This gives a 100% crash for me, running version 58.0.1.

Notes:
A) Typing about:privatebrowsing also repros.
B) You can also reproduce it by pasting either URL *twice* before switching tabs.
C) Creating a new tab at step 4 instead of switching to an existing one also repros.

/C
Device:
 - Huawei M3 Lite 10 Tablet (Android 7.0);
 
Build:   
  - Beta 60 (60.0b7);

Crash occurred once on Beta 60.0b7 when switching between 2 tabs. (first tab has about:config and the second about:buildconfig opened). 

Note:
A) Was not able to reproduce the crash again

b3bb391a-9aef-4c5e-9483-a7a300180327
Crash Signature: java.lang.IndexOutOfBoundsException: setSpan (23 ... 24) ends beyond length 23 at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)] [@ java.lang.IndexOutOfBoundsException: setSpan (29 ... 30) ends beyond length 29 at android.t… → java.lang.IndexOutOfBoundsException: setSpan (23 ... 24) ends beyond length 23 at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)] [@ java.lang.IndexOutOfBoundsException: setSpan (24 ... 27) ends beyond length 26 at android.t…
Both t24.com.tr and trt.net.tr are Turkish websites and crash occurs in keyboard component so it could be related to input language and beginning with a character equal or higher than "t"
Added a signature. Recent reports with this signature have a different stack compared to the one in comment 0, these are the top 10 frames:

java.lang.IndexOutOfBoundsException: setSpan (54 ... 55) ends beyond length 54
	at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java:1309)
	at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:680)
	at android.text.SpannableStringBuilder.setSpan(SpannableStringBuilder.java:672)
	at org.mozilla.gecko.home.BrowserSearch$9.format(BrowserSearch.java:1232)
	at org.mozilla.gecko.home.TwoLinePageRow.update(TwoLinePageRow.java:254)
	at org.mozilla.gecko.home.TwoLinePageRow.updateFromCursor(TwoLinePageRow.java:346)
	at org.mozilla.gecko.home.BrowserSearch$SearchAdapter.bindView(BrowserSearch.java:1203)
	at org.mozilla.gecko.home.MultiTypeCursorAdapter.getView(MultiTypeCursorAdapter.java:62)
	at android.widget.AbsListView.obtainView(AbsListView.java:3189)
        ...
Crash Signature: java.lang.IndexOutOfBoundsException: setSpan (59 ... 60) ends beyond length 59 at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java)] [@ java.lang.IndexOutOfBoundsException: setSpan (62 ... 63) ends beyond length 62 at android.t… → java.lang.IndexOutOfBoundsException: setSpan (54 ... 55) ends beyond length 54 at android.text.SpannableStringBuilder.checkRange(SpannableStringBuilder.java) ] [@ java.lang.IndexOutOfBoundsException: setSpan (59 ... 60) ends beyond length 59 at android.…
I know what's going on here and I might be able to cook up a fix: we search for a match within the title using strings that have been converted to lower case:

https://searchfox.org/mozilla-central/rev/0640ea80fbc8d48f8b197cd363e2535c95a15eb3/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#1219-1220

Later we use the match size on the original title:

https://searchfox.org/mozilla-central/rev/0640ea80fbc8d48f8b197cd363e2535c95a15eb3/mobile/android/base/java/org/mozilla/gecko/home/BrowserSearch.java#1232

This code assumes that the lower case version of a string obtained with String.toLowerCase() has the same length of the original string - which is true in many languages but not all. In some languages lowercase-to-uppercase conversions and uppercase-to-lowercase conversions might yield different character numbers. For example in German the uppercase of 'ß' is 'SS'.
I've got a fix in the works, taking this.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
This patch uses a case-insensitive matcher to highlight the title of a history
entry that's been typed by the user. Previously the matching substring was
calculated manually as lowercase assuming that it's representation would have
the same number of characters as the original mixed case. In some locales
howerver this assumption is wrong leading to out-of-bound exceptions when
highlighting part of the title.
Comment on attachment 9011442 [details]
Bug 1127855 - Handle title highlighting properly in all locales

Nick Alexander :nalexander [he/him] has approved the revision.
Attachment #9011442 - Flags: review+
Pushed by gsvelto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fbc27c2a3b88
Handle title highlighting properly in all locales r=nalexander
https://hg.mozilla.org/mozilla-central/rev/fbc27c2a3b88
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 64
Depends on: 1494203
Gabriele, is that fix a good candidate for a beta uplift? Thanks
Flags: needinfo?(gsvelto)
(In reply to Pascal Chevrel:pascalc from comment #21)
> Gabriele, is that fix a good candidate for a beta uplift? Thanks

Unfortunately not until we fix bug 1494203. I've got a patch ready there but it still needs to be reviewed. Once that bug is fixed we should be able to uplift both safely.
Flags: needinfo?(gsvelto)
Comment on attachment 9011442 [details]
Bug 1127855 - Handle title highlighting properly in all locales

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1366678

User impact if declined: Fennec will crash when typing characters in the awesomebar that have multi-character lower/upper-case equivalents (e.g. ß -> SS).

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: Bug 1494203

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The patch is a straighforward change of the affected code and the only issue that cropped up (bug 1494203) has already been fixed.

String changes made/needed: None
Attachment #9011442 - Flags: approval-mozilla-beta?
Gabriele, could you request uplift in Bug 1494203 as well please? Thanks
Flags: needinfo?(gsvelto)
Sure
Flags: needinfo?(gsvelto)
Comment on attachment 9011442 [details]
Bug 1127855 - Handle title highlighting properly in all locales

Uplift approved for 63 beta 13, thanks.
Attachment #9011442 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Assignee: gsvelto → nobody
Assignee: nobody → gsvelto
Product: Firefox for Android → Firefox for Android Graveyard

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

Keywords: steps-wanted
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: