Last Comment Bug 776547 - Crash [@ java.lang.NullPointerException: at org.mozilla.gecko.TextSelection$3.run(TextSelection.java)
: Crash [@ java.lang.NullPointerException: at org.mozilla.gecko.TextSelection$3...
Status: RESOLVED FIXED
[native-crash]
: crash, reproducible
Product: Firefox for Android
Classification: Client Software
Component: Text Selection (show other bugs)
: Trunk
: ARM Android
: -- critical (vote)
: Firefox 17
Assigned To: :Margaret Leibovic
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-23 08:24 PDT by Aaron Train [:aaronmt]
Modified: 2013-12-10 10:01 PST (History)
1 user (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
unaffected
fixed
fixed


Attachments
patch (2.59 KB, patch)
2012-07-23 10:47 PDT, :Margaret Leibovic
no flags Details | Diff | Splinter Review
create shared layout (9.13 KB, patch)
2012-07-23 13:31 PDT, :Margaret Leibovic
wjohnston2000: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Aaron Train [:aaronmt] 2012-07-23 08:24:33 PDT
This bug was filed from the Socorro interface and is 
report bp-c1cb660b-da22-4ce4-adb0-b39672120723 .
============================================================= 


E/GeckoAppShell( 3801): java.lang.NullPointerException
E/GeckoAppShell( 3801): 	at org.mozilla.gecko.TextSelection$3.run(TextSelection.java:56)
E/GeckoAppShell( 3801): 	at android.os.Handler.handleCallback(Handler.java:615)
E/GeckoAppShell( 3801): 	at android.os.Handler.dispatchMessage(Handler.java:92)
E/GeckoAppShell( 3801): 	at android.os.Looper.loop(Looper.java:137)
E/GeckoAppShell( 3801): 	at android.app.ActivityThread.main(ActivityThread.java:4745)
E/GeckoAppShell( 3801): 	at java.lang.reflect.Method.invokeNative(Native Method)
E/GeckoAppShell( 3801): 	at java.lang.reflect.Method.invoke(Method.java:511)
E/GeckoAppShell( 3801): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:786)
E/GeckoAppShell( 3801): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553)
E/GeckoAppShell( 3801): 	at dalvik.system.NativeStart.main(Native Method)
I/WindowState(  307): WIN DEATH: Window{41f28430 SurfaceView paused=false}
I/ActivityManager(  307): Process org.mozilla.fennec:org.mozilla.fennec.WebApp0 (pid 3801) has died.
W/ActivityManager(  307): Force removing ActivityRecord{423a5f90 org.mozilla.fennec/.WebApps$WebApp0}: app died, no saved state
I/WindowState(  307): WIN DEATH: Window{41f16b70 org.mozilla.fennec/org.mozilla.fennec.WebApps$WebApp0 paused=false}


STR:

i) http://testmanifest.com, and 'Install' a WebApp
ii) Launch WebApp
iii) Create a selection on the far left hand side text; "Logged in users can..."
--
Samsung Galaxy Nexus (Android 4.1.1)
Nightly (07/23)
Comment 1 :Margaret Leibovic 2012-07-23 08:36:40 PDT
I assume this is because the text selection views aren't included in the gecko layout that web apps use, since this crash means that the handle views are null:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java#1985

I'd think we just need to add the text selection layout to web_app.xml like:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/layout/gecko_app.xml#40
Comment 2 :Margaret Leibovic 2012-07-23 10:47:50 PDT
Created attachment 644977 [details] [diff] [review]
patch

This patch adds the text selection handle layouts to web_app.xml, but I also added a null check in the TextSelection constructor so that we won't crash if we run into this problem again with another GeckoApp instance (text selection just won't work in that case).
Comment 3 Wesley Johnston (:wesj) 2012-07-23 12:56:56 PDT
Comment on attachment 644977 [details] [diff] [review]
patch

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

I'd like to move the shared stuff into one layout so that we (hopefully) don't have to keep dealing with this. Clearing review.
Comment 4 :Margaret Leibovic 2012-07-23 13:31:14 PDT
Created attachment 645046 [details] [diff] [review]
create shared layout

The text_selection_handles layout is now only used in this shared layout. I feel like it's cleaner to keep it separate, but on the other hand it's nicer to minimize the number of XML files we have floating around. What do you think?
Comment 5 Wesley Johnston (:wesj) 2012-07-23 15:23:28 PDT
Comment on attachment 645046 [details] [diff] [review]
create shared layout

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

I like!

::: mobile/android/base/resources/layout/shared_ui_components.xml
@@ +1,5 @@
> +<?xml version="1.0" encoding="utf-8"?>
> +<!-- This Source Code Form is subject to the terms of the Mozilla Public
> +   - License, v. 2.0. If a copy of the MPL was not distributed with this
> +   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
> +

Lets add a quick comment here explaining what this file is.
Comment 7 :Margaret Leibovic 2012-07-23 15:49:52 PDT
Comment on attachment 645046 [details] [diff] [review]
create shared layout

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 774938 (native handles)
User impact if declined: webapps crash
Testing completed (on m-c, etc.): just landed on inbound
Risk to taking this patch (and alternatives if risky): low-risk inclusion of text selection handle layouts for webapps
String or UUID changes made by this patch: n/a
Comment 8 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-23 15:55:10 PDT
Comment on attachment 645046 [details] [diff] [review]
create shared layout

approved for beta as part of the dep bugs for native handles for text selection.
Comment 9 :Margaret Leibovic 2012-07-23 17:41:41 PDT
Actually, this doesn't need to be on beta because web apps aren't there. I landed it on aurora, though:
https://hg.mozilla.org/releases/mozilla-aurora/rev/6addc3c4408e

Note You need to log in before you can comment on or make changes to this bug.