Last Comment Bug 696282 - Implement native toast alert support
: Implement native toast alert support
Status: VERIFIED FIXED
[QA+]
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: P3 normal (vote)
: ---
Assigned To: Julien Vermet | JulienDev
:
Mentors:
Depends on:
Blocks: 697078
  Show dependency treegraph
 
Reported: 2011-10-20 17:51 PDT by [:fabrice] Fabrice Desré
Modified: 2012-01-09 11:36 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
11+


Attachments
Patch v1 (1.41 KB, patch)
2011-10-25 16:32 PDT, Julien Vermet | JulienDev
no flags Details | Diff | Review
Patch v2 (5.47 KB, patch)
2011-10-25 19:10 PDT, Julien Vermet | JulienDev
mark.finkle: review+
Details | Diff | Review

Description [:fabrice] Fabrice Desré 2011-10-20 17:51:55 PDT
We used to have two alert types:

- actionable ones, that map to Status Bar Notifications (http://developer.android.com/guide/topics/ui/notifiers/notifications.html)
- simple toasters, that map to Toast Notifications (http://developer.android.com/guide/topics/ui/notifiers/toasts.html)
Comment 1 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-20 18:07:23 PDT
We already support Status Bar notifications. We also have a "toaster" alertsservice, but it's XUL. Lets switch it over to use native toast by passing a JSON message from the JS component to the Java UI.
Comment 2 Julien Vermet | JulienDev 2011-10-25 16:32:14 PDT
Created attachment 569539 [details] [diff] [review]
Patch v1

This is a first implementation with two parameters :
- A JSONObject called "message" just contain a string called "content"
- A boolean to set the duration. If true, long, if false, short
Comment 3 Wesley Johnston (:wesj) 2011-10-25 16:41:16 PDT
Awesome. Thanks Julien.

I think you're digging at this a bit already, but what we'll probably want is a method like this, except maybe taking a String and a Bool.

Then we'll register a message listener, similar to the ones in embedding/android/GeckoApp.java (look for "registerGeckoEventListener", and note we'll also want to unregister the listener at close) that watches for "Toast" messages coming from Javascript. I think we should try to contain all the JSON conversions to that method.

So in the listener, we'll look at the JSON we get, pull out the message text and the "length" boolean (I think for the sake readability, we might be better making this a string with values "long" or "short"?), and then fire them over to the real "showToast" method.

Thanks again. Ask away if any of that doesn't make sense.
Comment 4 [:fabrice] Fabrice Desré 2011-10-25 16:51:39 PDT
Thanks for the patch Julien - let's build on top of this.

We want to call this from JS. It used to be an XPCOM component but let's start by just adding this to the NativeWindow object in browser.js around http://hg.mozilla.org/projects/birch/file/1f61c869b696/mobile/chrome/content/browser.js#l406 :
NativeWindow.toaster.show(aMessage, aLong)

Message type will be Toaster:Show, and this message must then be handled in Gecko.app:

registration: http://hg.mozilla.org/projects/birch/file/1f61c869b696/embedding/android/GeckoApp.java#l1087

handling : http://hg.mozilla.org/projects/birch/file/1f61c869b696/embedding/android/GeckoApp.java#l618

unregistration: http://hg.mozilla.org/projects/birch/file/1f61c869b696/embedding/android/GeckoApp.java#l1284
Comment 5 Julien Vermet | JulienDev 2011-10-25 19:10:25 PDT
Created attachment 569581 [details] [diff] [review]
Patch v2

As suggested by wesj, I used a string to set the toast duration and the JSON conversion is now done in "handleMessage". If the duration is "long", a long toast is shown. If anything else is defined, a short toast is shown.

A toast can be shown by calling the JS function "NativeWindow.toaster.show("myToast", "long");"
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-25 19:23:31 PDT
Comment on attachment 569581 [details] [diff] [review]
Patch v2

Very nice Julien!

This looks exactly like what we want. I'm going to do a few things to the patch before I land it:
1. Switch to  4-space indents, no Tabs
2. Use | } else if (...) { | format
3. Change the "length" param to "duration" - since that is what Android calls it

Thanks a lot for the patch
Comment 7 Mark Finkle (:mfinkle) (use needinfo?) 2011-10-25 19:53:40 PDT
https://hg.mozilla.org/projects/birch/rev/41404c189adc

I also tweak:
* "Toaster" -> "Toast" since Android calls it that too.
* Some other spacing issues _not_ in the patch itself (just OCD)
Comment 8 Andreea Pod 2011-11-29 02:25:06 PST
Saving a page as PDF two alerts appeared: Status Bar Notification and Toast Notification. Marking this as verified fixed. 

Build: Mozilla /5.0 (Android;Linux armv7l;rv:11.0a1) Gecko/20111128 Firefox/11.0a1 Fennec/11.0a1
Device: LG Optimus 2X (Android 2.2)

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