1248459 - When using "Find in page", crash in java.lang.IllegalArgumentException: invalid selection notification range

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-8ab2f95b-e1ec-4398-84a6-b58702160209.
=============================================================

There have been several new instances of this crash recently.

Comment 1

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

Attachment: Get more logs for debugging crash (v1)

In order to debug the crash, this patch turns on logging for GeckoEditable and
GeckoInputConnection for nightlies, and makes us fetch more lines of logcat
when submitting crash reports.

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/431dbfca93d0

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/431dbfca93d0

Comment 4

[Gian-Carlo Pascutto [:gcp]]

I have a 100% reproducible testcase for this on Aurora, on the RAZRi + SwiftKey:

1) Open https://wiki.mozilla.org/Modules/All
2) Press "Find In page" in the menu
3) Enter a single letter

Crashes 100% of the time.

Comment 5

[Gian-Carlo Pascutto [:gcp]]

I cannot reproduce this on Nightly, only on Aurora. But, I checked and the Aurora build is from the 15th, so it should have the fix from bug 1236705 already in it.

Comment 6

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

Attachment: Don't query out-of-bounds selection (v1)

This will fix the wiki crash.

During a query selection event, fail if the selection is outside of the
editor's root content. This can happen if the placeholder text in an
input field is somehow selected. The placeholder is in a separate
element outside of the root content.

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8721457 [details] [diff] [review]
Don't query out-of-bounds selection (v1)

Could you add automated test with synthesizeQuerySelectedText() in EventUtils.js? I'm not sure if this is right approach.

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I meant that it *might* be better to return [0-0] as the result.

Comment 9

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

Attachment: Add a test for querying out-of-bounds selection (v1)

Comment 10

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

(In reply to Masayuki Nakano [:masayuki] (Mozilla Japan) from comment #8)
> I meant that it *might* be better to return [0-0] as the result.

Perhaps. I think it's better to return failure because this is an edge case that should not happen normally.

Comment 11

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

Attachment: Initialize finder when showing find bar;

The other half of the equation: the finder component has a quirk that the first
time it searches for a string it may find text that's normally not included in
the search result (e.g. the placeholder text in an input element). I think
rather than fixing the quirk, which seems risky, it's better to work around it
by performing a fake dummy search when first showing the find bar.  On desktop,
we also (I think unintentionally) perform a dummy search when the find bar
first appears.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6aeedf17f577
https://hg.mozilla.org/integration/mozilla-inbound/rev/62d7f81e434a
https://hg.mozilla.org/integration/mozilla-inbound/rev/3271fbe16bbb

Comment 13

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

Backed out the first two patches for mochitest-chrome-3 failures:
https://treeherder.mozilla.org/logviewer.html#?job_id=22524656&repo=mozilla-inbound#L22259


https://hg.mozilla.org/integration/mozilla-inbound/rev/53003fb5d81a

Comment 14

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

Attachment: Don't query out-of-bounds selection (v1.1)

Added a fix in test_imestate so the test passes.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8be0c24081c6
https://hg.mozilla.org/integration/mozilla-inbound/rev/9993092fc9c8

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8be0c24081c6
https://hg.mozilla.org/mozilla-central/rev/9993092fc9c8

Comment 17

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

Comment on attachment 8726298 [details] [diff] [review]
Don't query out-of-bounds selection (v1.1)

Uplift request for just this patch.

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Random crashes when inputting text.
[Describe test coverage new/current, TreeHerder]: New testcase added.
[Risks and why]: Very small; the fix should have minimal impact.
[String/UUID change made/needed]: None

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8726298 [details] [diff] [review]
Don't query out-of-bounds selection (v1.1)

Android crash fix, please uplift to aurora. 
This should make it into the beta 1 build on Monday.

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

Jim, both patches should uplift, right?

Comment 20

[Christian Ascheberg]

I can still reproduce a crash in today's Nightly that links to this bug: bp-1bfb2476-44bf-48c1-954f-ca4e22160306

STR:
1. open a Github project site, issues or pull requests tab: https://github.com/mozilla/MozStumbler/issues
2. scroll to the bottom and choose "Desktop version"
3. click "Author" filter
4. input character or text into "Filter users" input
5. tapping enter on the keyboard crashes Fennec

Nexus 4 running latest stock Android

Comment 21

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

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #19)
> Jim, both patches should uplift, right?

Attachment 8726298 [details] [diff] and the testcase in attachment 8723386 [details] [diff] [review].

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8723386 [details] [diff] [review]
Add a test for querying out-of-bounds selection (v1)

This should land in beta 46 - no rush as it is just a testcase, it can go into beta 2.

Comment 23

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

(In reply to Christian Ascheberg from comment #20)
> I can still reproduce a crash in today's Nightly that links to this bug:
> bp-1bfb2476-44bf-48c1-954f-ca4e22160306

Thanks Christian! I'll make a separate bug for that cause of the crash.

Comment 24

[Carsten Book [:Tomcat]]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #18)
> Comment on attachment 8726298 [details] [diff] [review]
> Don't query out-of-bounds selection (v1.1)
> 
> Android crash fix, please uplift to aurora. 
> This should make it into the beta 1 build on Monday.

liz so this has beta approval too (since it only has aurora approval so far ?

Comment 25

[Liz Henry (:lizzard) (relman/hg->git project)]

tomcat I thought it landed before the merge early on Monday. looks like it didn't. So, yes in beta 46 please !

Comment 26

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8726298 [details] [diff] [review]
Don't query out-of-bounds selection (v1.1)

This should be on 46, looks like it may have missed the merge.

Comment 27

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

https://hg.mozilla.org/releases/mozilla-beta/rev/76bb8e1cef63
https://hg.mozilla.org/releases/mozilla-beta/rev/a00dbc1e2184

Comment 28

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

Attachment: Turn off diagnostic logging; r=me

Turn off diagnostic logging because this bug has been fixed.

Comment 29

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d668b4c53f69

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d668b4c53f69