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

RESOLVED FIXED in Firefox 46

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jchen, Assigned: jchen)

Tracking

({crash})

unspecified
Firefox 47
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed, firefox47 fixed)

Details

(crash signature)

Attachments

(5 attachments, 1 obsolete attachment)

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.
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.
Attachment #8719560 - Flags: review?(snorp)
Attachment #8719560 - Flags: review?(snorp) → review+
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.
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.
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.
Attachment #8721457 - Flags: review?(masayuki)
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.
Flags: needinfo?(nchen)
I meant that it *might* be better to return [0-0] as the result.
Depends on: 1250314
(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.
Flags: needinfo?(nchen)
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.
Attachment #8723387 - Flags: review?(margaret.leibovic)

Updated

3 years ago
Attachment #8723387 - Flags: review?(margaret.leibovic) → review+
Flags: needinfo?(nchen)
Added a fix in test_imestate so the test passes.
Attachment #8726298 - Flags: review+
Attachment #8721457 - Attachment is obsolete: true
Flags: needinfo?(nchen)
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
Attachment #8726298 - Flags: approval-mozilla-aurora?
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.
Attachment #8726298 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Jim, both patches should uplift, right?
Flags: needinfo?(nchen)
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
(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].
Flags: needinfo?(nchen)
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.
Attachment #8723386 - Flags: approval-mozilla-beta+
(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.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Keywords: leave-open
Hardware: Unspecified → All
Resolution: --- → FIXED
Summary: crash in java.lang.IllegalArgumentException: invalid selection notification range at org.mozilla.gecko.GeckoEditable.onSelectionChange(GeckoEditable.java) → When using "Find in page", crash in java.lang.IllegalArgumentException: invalid selection notification range
Target Milestone: --- → Firefox 47
(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 ?
Flags: needinfo?(lhenry)
tomcat I thought it landed before the merge early on Monday. looks like it didn't. So, yes in beta 46 please !
Flags: needinfo?(lhenry)
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.
Attachment #8726298 - Flags: approval-mozilla-beta+
Turn off diagnostic logging because this bug has been fixed.
Attachment #8739154 - Flags: review+
You need to log in before you can comment on or make changes to this bug.