Last Comment Bug 781127 - [b2g][Web Activities][System Message Handler] mozChromeEvent to System app does not expose enough information
: [b2g][Web Activities][System Message Handler] mozChromeEvent to System app do...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: [:fabrice] Fabrice Desré
: Jason Smith [:jsmith]
Mentors:
: 775183 (view as bug list)
Depends on: 715814 system-message-api 783323
Blocks: 782488 782489
  Show dependency treegraph
 
Reported: 2012-08-08 04:00 PDT by Tim Guan-tin Chien [:timdream] (please needinfo)
Modified: 2012-09-14 18:22 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
?
+


Attachments
patch (2.99 KB, patch)
2012-08-08 14:19 PDT, [:fabrice] Fabrice Desré
21: review-
Details | Diff | Splinter Review
patch v2 (2.98 KB, patch)
2012-08-13 17:22 PDT, [:fabrice] Fabrice Desré
21: review+
Details | Diff | Splinter Review

Description Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-08 04:00:19 PDT
We have a few occasions that System Message Handler need to dispatch a type: 'open-app' mozChromeEvent to the System app.

- Alarm timeout
- Web Activity (disposition: window)
- Web Activity (disposition: inline)

It is very important for the System apps (specifically, window manager) to differentiate these three type of 'open-app' requests

- Alarm timeout -> launch app in background
- Web Activity (disposition: window) -> switch window to specified app
- Web Activity (disposition: inline) -> don't switch to the app, but put the app over the current app; it will be dismissed when the transaction is complete.

without these information we are blocked from implementing this

https://github.com/mozilla-b2g/gaia/issues/3245
Comment 1 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-08 04:04:20 PDT
Unset blocking flag to ?
Comment 2 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-08 04:20:55 PDT
without this, we are also bring Clock app to the foreground every time alarm fires, which is not desired.
Comment 3 [:fabrice] Fabrice Desré 2012-08-08 14:19:50 PDT
Created attachment 650310 [details] [diff] [review]
patch

Tim,

Can you try with this patch?

I added two fields to the event.detail that you get in content:
- |evt.detail.message| is the message name ('alarm', 'activity', ...)

- |evt.detail.target| is the description of the chosen activity. You can get the disposition in this property, that will look like:
"target":{"filters":{"type":"websms/sms"},"disposition":"window"}
Comment 4 Andrew Overholt [:overholt] 2012-08-10 09:53:10 PDT
Josh, we'd like some UX input on comment #2.
Comment 5 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-08-13 17:05:03 PDT
Comment on attachment 650310 [details] [diff] [review]
patch

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

::: b2g/chrome/content/shell.js
@@ +377,5 @@
>      type: 'open-app',
>      url: msg.uri,
>      origin: origin,
> +    manifest: msg.manifest,
> +    message: msg.type,

I'm not convinced the |message| field is needed. 

Since the default disposition for an Activity is |window|, is it possible to always set the field for activities and other system message emitters can set the target value to what they want? That would let Gaia differentiate between the disposition: inline|window versus no disposition.
Comment 6 [:fabrice] Fabrice Desré 2012-08-13 17:22:34 PDT
Created attachment 651583 [details] [diff] [review]
patch v2

Same one without the |message| field in the chromeEvent.
Comment 7 [:fabrice] Fabrice Desré 2012-08-15 09:01:01 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/cfd10c640375
Comment 8 Chris Lee [:clee] 2012-08-15 14:02:45 PDT
Josh, can we just use the notification UI that we're building?  

I would say this is a blocker for cases like the Alarm clock.
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-08-15 18:46:03 PDT
https://hg.mozilla.org/mozilla-central/rev/cfd10c640375
Comment 10 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-16 03:19:10 PDT
(In reply to Vivien Nicolas (:vingtetun) from comment #5)
> Comment on attachment 650310 [details] [diff] [review]
> patch
> 
> Review of attachment 650310 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: b2g/chrome/content/shell.js
> @@ +377,5 @@
> >      type: 'open-app',
> >      url: msg.uri,
> >      origin: origin,
> > +    manifest: msg.manifest,
> > +    message: msg.type,
> 
> I'm not convinced the |message| field is needed. 
> 
> Since the default disposition for an Activity is |window|, is it possible to
> always set the field for activities and other system message emitters can
> set the target value to what they want? That would let Gaia differentiate
> between the disposition: inline|window versus no disposition.

I may be to late :( we would need the |message| field to tell if this is a activity request or not first. Without |message| we cannot handle the alarm use case :-/
Comment 11 Josh Carpenter [:jcarpenter] 2012-08-16 05:00:59 PDT
Hi guys, sorry for late response; still catching up.

> [Andrew] Josh, we'd like some UX input on comment #2.

If I understand correctly, in absence of System Message Handler being capable of sending an 'open-app' mozChromeEvent, we cannot do the temporary "Entry sheet" overlays via disposition:inline, and instead all Activity UX must be full app switch.

From skimming comments since Aug 8, it looks like this has already been addressed. If not, it's a very strong UX blocking-basecamp +. We use disposition:inline frequently. It enables us to bring functionality _to_ the user, instead of sending them to random apps.

> Josh, can we just use the notification UI that we're building?  

Maybe. When device is unclocked, Active alarms could display and ring via a standard Notification, which when pressed would open the Alarm app, which in turn would display the Active Alarm screen. However I'm not quite sure how that approach would work when the device was locked (which is when most Alarms go off). Our Lock screen Notifications are simpler (cannot access app directly), and can be hidden entirely by the user from Settings > Notifications. I don't think this approach would work without modifying our Lock screen notifications.

> I may be to late :( we would need the |message| field to tell if this is a activity request or not first. Without |message| we cannot handle the alarm use case :-/

More is better. UX will find a use for it ;)
Comment 12 Josh Carpenter [:jcarpenter] 2012-08-16 05:02:01 PDT
> Status: NEW → RESOLVED
> Resolution: --- → FIXED

Reading is hard at 5:00 am. Yawn.

I take it then that this has been fixed?
Comment 13 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-08-16 10:52:55 PDT
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #10)
> (In reply to Vivien Nicolas (:vingtetun) from comment #5)
> > Comment on attachment 650310 [details] [diff] [review]
> > patch
> > 
> > Review of attachment 650310 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: b2g/chrome/content/shell.js
> > @@ +377,5 @@
> > >      type: 'open-app',
> > >      url: msg.uri,
> > >      origin: origin,
> > > +    manifest: msg.manifest,
> > > +    message: msg.type,
> > 
> > I'm not convinced the |message| field is needed. 
> > 
> > Since the default disposition for an Activity is |window|, is it possible to
> > always set the field for activities and other system message emitters can
> > set the target value to what they want? That would let Gaia differentiate
> > between the disposition: inline|window versus no disposition.
> 
> I may be to late :( we would need the |message| field to tell if this is a
> activity request or not first. Without |message| we cannot handle the alarm
> use case :-/

Is |target| not enough? |disposition| should be undefined or null for system messages that does not originate from an activity.
Comment 14 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-16 10:56:01 PDT
(In reply to Vivien Nicolas (:vingtetun) from comment #13)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #10)
> > (In reply to Vivien Nicolas (:vingtetun) from comment #5)
> > > 
> > > I'm not convinced the |message| field is needed. 
> > > 
> > > Since the default disposition for an Activity is |window|, is it possible to
> > > always set the field for activities and other system message emitters can
> > > set the target value to what they want? That would let Gaia differentiate
> > > between the disposition: inline|window versus no disposition.
> > 
> > I may be to late :( we would need the |message| field to tell if this is a
> > activity request or not first. Without |message| we cannot handle the alarm
> > use case :-/
> 
> Is |target| not enough? |disposition| should be undefined or null for system
> messages that does not originate from an activity.

No, System Message Handler is used by both Alarm API and Web Activities to send messages to apps, and possibility a lot more other messages in the future. We need to know the message we are handling.
Comment 15 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-16 10:58:30 PDT
(In reply to Vivien Nicolas (:vingtetun) from comment #13)
> Is |target| not enough? |disposition| should be undefined or null for system
> messages that does not originate from an activity.

The |target| property simply contains object copied from the manifest. For activities handler that doesn't specify disposition, there is no |disposition| property. Rely on that is probably not a good idea.
Comment 16 Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) 2012-08-16 11:15:13 PDT
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #15)
> (In reply to Vivien Nicolas (:vingtetun) from comment #13)
> > Is |target| not enough? |disposition| should be undefined or null for system
> > messages that does not originate from an activity.
> 
> The |target| property simply contains object copied from the manifest. For
> activities handler that doesn't specify disposition, there is no
> |disposition| property. Rely on that is probably not a good idea.

|disposition| seems more relevant to me than 'alarm' or possibly 'sms', 'telephony', and adding more and more cases. If I understand correctly what you try to resolve this is to to launch the alarm application in the background instead of the foreground? In this case a generic |disposition: background| for such messages sounds more relevant. Will it be ok for you?
Comment 17 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-08-16 11:36:31 PDT
(In reply to Vivien Nicolas (:vingtetun) from comment #16)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #15)
> > (In reply to Vivien Nicolas (:vingtetun) from comment #13)
> > > Is |target| not enough? |disposition| should be undefined or null for system
> > > messages that does not originate from an activity.
> > 
> > The |target| property simply contains object copied from the manifest. For
> > activities handler that doesn't specify disposition, there is no
> > |disposition| property. Rely on that is probably not a good idea.
> 
> |disposition| seems more relevant to me than 'alarm' or possibly 'sms',
> 'telephony', and adding more and more cases. If I understand correctly what
> you try to resolve this is to to launch the alarm application in the
> background instead of the foreground? In this case a generic |disposition:
> background| for such messages sounds more relevant. Will it be ok for you?

That would also make sense. Except, Gecko would have no idea what kind of |disposition| it should tell System app when it is sending a 'alarm' message, or it should not have an idea at all since Gecko's role here is simply an information rely, it's logic should not involve presentation.

If we are going with this direction, then |disposition| should be under |detail| directly. It's not just something contain in the Web Activity message anymore.
Comment 18 Gene Lian [:gene] (I already quit Mozilla) 2012-09-05 21:42:05 PDT
*** Bug 775183 has been marked as a duplicate of this bug. ***

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