+++ This bug was initially created as a clone of Bug #779176 +++ Discovered while working on https://bugzilla.mozilla.org/show_bug.cgi?id=782488 The easiest STR is to take this branch https://github.com/etiennesegonzac/gaia/tree/dialer-minus-bg-service On incoming call the "Communications" app is launched but the system message handler (in dialer/js/dialer.js) is never triggered (should log in logcat) But once the app is launched, it works. Going back to a gaia version where the Dialer is a standalone app it works perfectly (the app is launched and the system message is triggered).

Well the system message glue is asking gaia to launch |app://communications.gaiamobile.org/| and not |app://communications.gaiamobile.org/dialer/| so the system message handler is never *registered* (and obviously never called). We get around this for WebActivities with the |href| property. What can we do here?

(In reply to Etienne Segonzac from comment #1) > Well the system message glue is asking gaia to launch > |app://communications.gaiamobile.org/| and not > |app://communications.gaiamobile.org/dialer/| so the system message handler > is never *registered* (and obviously never called). > > We get around this for WebActivities with the |href| property. > > What can we do here? Should we register the system message adding the href when a activity declares it? Or is totally out of the scope of WebActivities? Thanks!

IIRC, when we worked on this API with Jonas, the idea was that the app manifest could point which URL should be opened when a specific system message was received. I do not remember if that was implemented so it's not clear to me if the issue is the lack of this being implemented or a bug in that implementation.

(In reply to Mounir Lamouri (:mounir) from comment #3) > IIRC, when we worked on this API with Jonas, the idea was that the app > manifest could point which URL should be opened when a specific system > message was received. Sounds good. I'll check with Fabrice.

Allowing an alternative syntax: ``` "messages": [ { "message": "telephony-incoming", "href": "/dialer/index.html" } ] ``` To declare a system message handler with a href param.

(In reply to Etienne Segonzac from comment #5) > Created attachment 658105 [details] [diff] [review] > POC > > Allowing an alternative syntax: > ``` > "messages": [ > { > "message": "telephony-incoming", > "href": "/dialer/index.html" > } > ] > ``` > > To declare a system message handler with a href param. Could we do something like: "messages: [ "foo": { href: "bar.html" }, "bar", "foobar" }

We could add the href to the |messages| property indeed, but I think that what apps dev actually want is to bind a message to an entry point. So another option is to allow |messages| in entry_points and use the launch_path of the entry point. This also applies to web activities.

Implementing the new syntax discussed: "messages": [ { "telephony-incoming": { "href": "/dialer/index.html" } } ]

(In reply to Fabrice Desré [:fabrice] from comment #7) > We could add the href to the |messages| property indeed, but I think that > what apps dev actually want is to bind a message to an entry point. So > another option is to allow |messages| in entry_points and use the > launch_path of the entry point. This also applies to web activities. Interesting... I don't really have a strong opinion one way or the other. Mounir?

Fabrice says this is a nice to have and not a v1 blocker.

Well #782488 is a basecamp+ and we cannot do the Gaia part of the change without this one...

Yeah, my bad. I misread that as a web activities issues. That should block.

This patch implements both the syntax from comment #8 and the entry points idea. I also update the activities code to let authors declare activities in entry points. Etienne, can you test with you gaia branch?

Awesome! Tested with the |messages| field in the |entry_points|, working well. No luck with the syntax from comment #8, didn't investigate further.

updated patch, that supports a slightly different syntax: "messages": [ { "telephony-incoming": "/dialer/index.html" }, "alarm" ] I like the compactness of this syntax. Etienne, let me know if that's good enough for you.

(In reply to Fabrice Desré [:fabrice] from comment #15) > Created attachment 659790 [details] [diff] [review] > patch v2 > > updated patch, that supports a slightly different syntax: > > "messages": [ > { "telephony-incoming": "/dialer/index.html" }, > "alarm" > ] > > I like the compactness of this syntax. Etienne, let me know if that's good > enough for you. Sounds good to me.

(In reply to Fabrice Desré [:fabrice] from comment #15) > Created attachment 659790 [details] [diff] [review] > I like the compactness of this syntax. Etienne, let me know if that's good > enough for you. Looks good. Do you want me to test the patch?

Both syntaxes are working flawlessly!

Comment on attachment 659790 [details] [diff] [review] patch v2 Mounir, Vivien: whoever can do this review, thank you!

Comment on attachment 659790 [details] [diff] [review] patch v2 Review of attachment 659790 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/apps/src/Webapps.jsm @@ +132,5 @@ > + _registerSystemMessagesForEntryPoint: function(aManifest, aApp, aEntryPoint) { > + let root = aManifest; > + if (aEntryPoint && aManifest.entry_points[aEntryPoint]) { > + root = aManifest.entry_points[aEntryPoint]; > + } Do you really need aEntryPoint as parameter or can you replace aManifest directly from the caller? I just feel like some work is duplicated but i can be wrong.

Comment on attachment 659790 [details] [diff] [review] patch v2 Review of attachment 659790 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/apps/src/Webapps.jsm @@ +154,5 @@ > } > }, > > + _registerSystemMessages: function(aManifest, aApp) { > + this._registerSystemMessagesForEntryPoint(aManifest, aApp, null); Isn't this going to register all system messages on the root? @@ +199,5 @@ > } > }, > > + _registerActivities: function(aManifest, aApp) { > + this._registerActivitiesForEntryPoint(aManifest, aApp, null); ditto for activities.

Minor changes in this patch: - a couple of comments - fixing the system messages and activities registration in confirmInstall - doing an early return in _registerSystemMessagesForEntryPoint > ::: dom/apps/src/Webapps.jsm > @@ +132,5 @@ > > + _registerSystemMessagesForEntryPoint: function(aManifest, aApp, aEntryPoint) { > > + let root = aManifest; > > + if (aEntryPoint && aManifest.entry_points[aEntryPoint]) { > > + root = aManifest.entry_points[aEntryPoint]; > > + } > > Do you really need aEntryPoint as parameter or can you replace aManifest > directly from the caller? I just feel like some work is duplicated but i can > be wrong. Unfortunately I need it to get the fullLaunchPath... > > > > + _registerSystemMessages: function(aManifest, aApp) { > > + this._registerSystemMessagesForEntryPoint(aManifest, aApp, null); > > Isn't this going to register all system messages on the root? yes, this will register system messages declared on the root of the manifest, which is what we want. Declaring them in the entry_point is just a convenience for authors.