Closed Bug 736312 Opened 9 years ago Closed 9 years ago

navigator.mozApps support on b2g


(Firefox OS Graveyard :: General, defect)

Gonk (Firefox OS)
Not set


(Not tracked)



(Reporter: fabrice, Assigned: fabrice)



(2 files, 1 obsolete file)

Attached patch patch (obsolete) — Splinter Review
Now that the new API has landed on m-c, we can add the glue to support it on b2g.

This patch enables the API, and maanges the launch() call by dispatching a "webapps-launch: event to the homescreen.
Attachment #606402 - Flags: review?(21)
Attached file test html file
Comment on attachment 606402 [details] [diff] [review]

>diff --git a/b2g/chrome/content/shell.js b/b2g/chrome/content/shell.js

>+WebappsHelper = {
>+  init: function webapps_init() {
>+    Services.obs.addObserver(this, "webapps-launch", false);

The rest of the file use ' instead of "

>+  },
>+  observe: function webapps_observe(aSubject, aTopic, aData) {

the rest of the file do not append the 'a' prefix

>+    let data = JSON.parse(aData);
>+    DOMApplicationRegistry.getManifestFor(data.origin, (function(aManifest) {
>+      if (!aManifest)
>+        return;

nit: add a line break

>+      let manifest = new DOMApplicationManifest(aManifest, data.origin);
>+      shell.sendEvent(content, "webapps-launch", { url: manifest.fullLaunchPath(), origin: data.origin } );

This is the stuff I'm not comfortable with. Is "webapps-launch" event describe somewhere on a spec? I would like to avoid to introduce 'proprietary' events.

>+    }).bind(this));

I don't think you need bind.
We don't have a spec defining how content is supposed to handle launching web apps.  We get to define it.

Not too thrilled with the approach here but it works for a v0.
Attached patch patch v2Splinter Review
updated patch according to IRC discussion
Attachment #606402 - Attachment is obsolete: true
Attachment #606402 - Flags: review?(21)
Attachment #606636 - Flags: review?(21)
Comment on attachment 606636 [details] [diff] [review]
patch v2

r+ if you fix the comments/nits I have done in the previous comment.
Attachment #606636 - Flags: review?(21) → review+
Assignee: nobody → fabrice
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.