Last Comment Bug 786338 - NotificationHandler sending bad intents to gecko
: NotificationHandler sending bad intents to gecko
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Wesley Johnston (:wesj)
:
:
Mentors:
: 786394 787425 789347 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-28 10:15 PDT by Wesley Johnston (:wesj)
Modified: 2016-07-29 14:29 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
unaffected
+
fixed
fixed


Attachments
Patch (1.08 KB, patch)
2012-08-28 11:36 PDT, Wesley Johnston (:wesj)
mark.finkle: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta-
Details | Diff | Splinter Review

Description Wesley Johnston (:wesj) 2012-08-28 10:15:23 PDT
Margaret has been seeing some strange behavior with notifications potentially. i.e. a "This url must be opened by an app Alert prompt appears" with nothing to do. I recently changed their behavior slightly in bug 781061. It sounds like we are trying to open the dat url sent to Fennec rather than The intent being sent to Fennec is what I expect:

I/GeckoNotificationHandler(25156): startActivity with intent: Action='org.mozilla.gecko.ACTION_ALERT_CALLBACK appName='org.mozilla.fennec.App'
I/ActivityManager(  308): START {act=org.mozilla.gecko.ACTION_ALERT_CALLBACK dat=alert:/-297100515?name=update-app&app=org.mozilla.fennec.App&cookie= flg=0x10000000 cmp=org.mozilla.fennec/.App u=0} from pid 25156

I'm guessing that we are trying to open this dat term when the Notification is tapped and Gecko isn't running. Will need to confirm that though.
Comment 1 Wesley Johnston (:wesj) 2012-08-28 11:36:39 PDT
Created attachment 656127 [details] [diff] [review]
Patch

I hate having to do this this way. If Gecko is running and you tap a Notification, we end up calling onNewIntent which has all this code to do special things with different intents. If not (we used to just bail, but with my recent change we start Gecko now and) we call onCreate which just assumes any url's it finds in an intent are real urls.

I'd really like onCreate to call into the same code that onNewIntent does. Unfortunately, onNewIntent fires intents to start loads whereas onCreate just sends the url to gecko with startup params. i.e. untangling the two is not being easy. Since this is on beta right now, I'd like to put up this patch for there and work on a better one for Nightly?
Comment 2 :Margaret Leibovic 2012-08-28 13:14:44 PDT
Oh, I didn't see you file this. I filed bug 786394. Is that a dupe now?
Comment 3 Wesley Johnston (:wesj) 2012-08-28 13:18:57 PDT
Yeah. I'm still not sure what your webapp bug was though....

*** This bug has been marked as a duplicate of bug 786394 ***
Comment 5 Wesley Johnston (:wesj) 2012-08-29 08:56:17 PDT
Comment on attachment 656127 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 781061
User impact if declined: Clicking system Notifications after Fennec has closed will lead to strange messages and blank tabs. In webapps it can lead to unusable situations (i.e. a blank tab you can't get out of, but webapps are turned off on beta).
Testing completed (on m-c, etc.): Landed on mc 8-29
Risk to taking this patch (and alternatives if risky): This is the low risk alternative. We could change 781061 so that we don't launch fennec in these situations again, but that requires some Android trickery that is non-trivial on Aurora. We could revert 781061 entirely on Beta since webapps are disabled there.
String or UUID changes made by this patch: None.
Comment 6 Alex Keybl [:akeybl] 2012-08-29 15:47:10 PDT
Let's track the backout of bug 781061 there.
Comment 7 Alex Keybl [:akeybl] 2012-08-29 15:47:38 PDT
*** Bug 786394 has been marked as a duplicate of this bug. ***
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-08-29 17:18:30 PDT
https://hg.mozilla.org/mozilla-central/rev/1052d9e756f8
Comment 9 Wesley Johnston (:wesj) 2012-08-31 09:10:19 PDT
*** Bug 787425 has been marked as a duplicate of this bug. ***
Comment 10 Aaron Train [:aaronmt] 2012-09-06 19:06:07 PDT
*** Bug 789347 has been marked as a duplicate of this bug. ***
Comment 11 Aaron Train [:aaronmt] 2012-09-06 19:06:35 PDT
Aurora checkin?
Comment 12 Wesley Johnston (:wesj) 2012-09-12 09:51:43 PDT
Sorry for the delay:
https://hg.mozilla.org/releases/mozilla-aurora/rev/d164700915f8
Comment 13 Cristian Nicolae (:xti) 2012-09-14 05:53:22 PDT
Did the patch land on Aurora 09/13?

I installed the Aurora build mentioned above, I checked for updates and I quit app after the notification was triggered. Clearing the notifications will auto-open Aurora.
Comment 14 Cristian Nicolae (:xti) 2012-09-17 02:50:30 PDT
I can reproduce this issue on the Aurora from 09/14. When notification update was triggered, I quit Aurora and then I cleared the update notification from Android Notification Bar. The result was auto-app relaunch. Reopening bug

--
Device: Galaxy Note
OS: Android 4.0.4
Comment 15 Wesley Johnston (:wesj) 2012-09-17 08:30:22 PDT
Ahh. The app relaunching is a different bug than the one fixed here (trying to open a bogus URL). The other piece was fixed in bug 786599, but not pushed to Aurora yet (doing that now). Thanks for catching that. Reclosing.

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