1384829 - Crash in java.lang.IllegalArgumentException: invalid span offsets at org.mozilla.gecko.GeckoEditable$Action.newSetSpan(GeckoEditable.java)

Status: RESOLVED FIXED

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

Description

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

This bug was filed from the Socorro interface and is 
report bp-8dde9949-40bd-49b2-8ae0-8225e0170726.
=============================================================

Crash reports seem to indicate the crash happens when using physical keyboards; needs more investigation.

Comment 1

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

Attachment: Bug 1384829 - 1. Always perform shadow text actions;

We want to always perform actions on the shadow text side even if a
particular GeckoEditable instance is disconnected from Gecko, because
there could be other users of Editable that still expect the object to
perform valid actions.

Review commit: https://reviewboard.mozilla.org/r/184392/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184392/

Comment 2

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

Attachment: Bug 1384829 - 2. Add more user-friendly debugging output;

Translate non-ASCII characters into hex instead of trying to print them
out.

Review commit: https://reviewboard.mozilla.org/r/184394/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184394/

Comment 3

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

Attachment: Bug 1384829 - 3. No longer ignore certain exceptions;

Bug 817386 added code to ignore IndexOutOfBoundsException when using
GeckoEditable because the code wasn't mature enough back then, and there
were many race conditions. I think the situation is a lot better now, so
we can try removing that code and see if we still need it. We can always
add it back if we do.

Review commit: https://reviewboard.mozilla.org/r/184396/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184396/

Comment 4

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

Attachment: Bug 1384829 - 4. Add test case to testInputConnection;

Add a test case to testInputConnection that makes sure GeckoEditable's
Editable interface still behaves correctly even after disconnecting from
Gecko due to a blur.

Review commit: https://reviewboard.mozilla.org/r/184398/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184398/

Comment 5

[Eugen Sawin [:esawin]]

Comment on attachment 8913029 [details]
Bug 1384829 - 1. Always perform shadow text actions;

https://reviewboard.mozilla.org/r/184392/#review189764

Comment 6

[Eugen Sawin [:esawin]]

Comment on attachment 8913030 [details]
Bug 1384829 - 2. Add more user-friendly debugging output;

https://reviewboard.mozilla.org/r/184394/#review189768

Comment 7

[Eugen Sawin [:esawin]]

Comment on attachment 8913031 [details]
Bug 1384829 - 3. No longer ignore certain exceptions;

https://reviewboard.mozilla.org/r/184396/#review189770

Comment 8

[Eugen Sawin [:esawin]]

Comment on attachment 8913032 [details]
Bug 1384829 - 4. Add test case to testInputConnection;

https://reviewboard.mozilla.org/r/184398/#review189774

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e2c72495a1b
1. Always perform shadow text actions; r=esawin
https://hg.mozilla.org/integration/autoland/rev/5c83f8313d0b
2. Add more user-friendly debugging output; r=esawin
https://hg.mozilla.org/integration/autoland/rev/c6a68e0e09b2
3. No longer ignore certain exceptions; r=esawin
https://hg.mozilla.org/integration/autoland/rev/f7ca10f036de
4. Add test case to testInputConnection; r=esawin

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6e2c72495a1b
https://hg.mozilla.org/mozilla-central/rev/5c83f8313d0b
https://hg.mozilla.org/mozilla-central/rev/c6a68e0e09b2
https://hg.mozilla.org/mozilla-central/rev/f7ca10f036de

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Given the volume, I think we definitely want this on Beta. I'm almost wondering if we should keep this in mind as a dot release ride-along candidate too. >5000 crashes with this signature in the last week on Fx55.

Comment 14

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

Comment on attachment 8913029 [details]
Bug 1384829 - 1. Always perform shadow text actions;

Uplift request for part 1 and part 4. (part 2 is debugging only and part 3 is riskier and not required to fix this crash)

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: Possible random crashes when inputting text
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Minimal risk
[Why is the change risky/not risky?]: Patch introduces a small amount of new code but it's covered by new and existing tests.
[String changes made/needed]: None

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8913029 [details]
Bug 1384829 - 1. Always perform shadow text actions;

Fixes a top crash, beta57+

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/77a84867b31b
https://hg.mozilla.org/releases/mozilla-beta/rev/52db05e860d8

Comment 17

[Sylvestre Ledru [:Sylvestre]]

We already ship 55 with this, we will let this ride the train from 57.

Comment 18

[u549602]

Could not reproduce this issue in our Beta test session today on 57.0b7.
Leaving this bug unverified until we get more crash stats information