Closed
Bug 1155758
Opened 10 years ago
Closed 10 years ago
Make about:serviceworkers work in B2G
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
Tracking
(firefox40 fixed)
RESOLVED
FIXED
2.2 S12 (15may)
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: ferjm, Assigned: ferjm)
References
Details
(Whiteboard: [s1])
Attachments
(3 files, 2 obsolete files)
No description provided.
Assignee | ||
Updated•10 years ago
|
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
It seems that the approach I was following here is not going to work :(
As you can see in the attached patches, I was trying to load app://system.gaiamobile.org/about_service_workers.html when loading about:serviceworkers from the Browser app. I was hoping to access the ServiceWorkerManager information from the System app via content/chrome event messaging, but even if the about_service_workers.html page is part of the System app, it is executed in the context of the Browser app (as a child iframe) and we cannot communicate with the platform from there (unless we expose ServiceWorkerManager to content, which it's quite unlikely to happen).
So instead of this, I believe that we can show this information on the developers menu within the Settings app. I can still use the System app to access the ServiceWorkerManager information and I can expose an IAC based API to Settings so it can consume this data.
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8596758 -
Attachment is obsolete: true
Assignee | ||
Comment 5•10 years ago
|
||
This is how the settings panel looks
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8599949 -
Attachment is obsolete: true
Attachment #8600940 -
Flags: review?(fabrice)
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8596756 [details]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master
Alive, could you review the System app bits, please? Arthur, could you take a look at the Settings app ones, please?
Thanks!
Attachment #8596756 -
Flags: review?(arthur.chen)
Attachment #8596756 -
Flags: review?(alive)
Comment 8•10 years ago
|
||
Comment on attachment 8600940 [details] [diff] [review]
v1
Review of attachment 8600940 [details] [diff] [review]:
-----------------------------------------------------------------
r=me with nits fixed.
::: b2g/components/AboutServiceWorkers.jsm
@@ +21,5 @@
> +XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
> + "resource://gre/modules/NetUtil.jsm");
> +
> +function debug(aMsg) {
> + //dump('AboutServiceWorkers - ' + aMsg + '\n');
nit: you started this file with double quotes, which I approve ;) let's go on! (there are other single quotes later too).
@@ +31,5 @@
> + }
> +
> + let result = {};
> +
> + Object.keys(aServiceWorkerInfo).forEach((property) => {
nit: forEach(property => {...
@@ +85,5 @@
> + let message = aEvent.detail;
> +
> + debug('Got content event ' + JSON.stringify(message));
> +
> + if (!message.id || !message.name) {
maybe console.log() a warning?
@@ +101,5 @@
> + };
> +
> + let data = gServiceWorkerManager.getAllRegistrations();
> + if (!data) {
> + sendError(message.id, "NO_SERVICE_WORKERS_REGISTRATIONS");
nit: can we change the errors to CamelCase instead?
@@ +113,5 @@
> + enabled: this.enabled,
> + registrations: registrations
> + });
> + return;
> + }
I think it's fine to remove this block this the next for() loop will just not be entered.
@@ +149,5 @@
> + return;
> + }
> +
> + let principal = Services.scriptSecurityManager.getAppCodebasePrincipal(
> + NetUtil.newURI(message.principal.origin),
could be Services.io.newURI(message.principal.origin, null, null) so we would not need NetUtil.jsm
Attachment #8600940 -
Flags: review?(fabrice) → review+
Comment 9•10 years ago
|
||
Comment on attachment 8596756 [details]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master
Looks fine, only naming nits
Attachment #8596756 -
Flags: review?(alive) → review+
Assignee | ||
Updated•10 years ago
|
Whiteboard: [s1]
Comment 10•10 years ago
|
||
Do we need UI review on this? I don't think Service Workers sounds friendly to ordinary users.
Our UI should at least explain these workers might consume data and/or battery at the background, for example.
Flags: needinfo?(firefoxos-ux-bugzilla)
Assignee | ||
Comment 11•10 years ago
|
||
Well, this is a panel inside the developers menu that "ordinary" users are probably not interested in. Do we want to explain all the details and panels inside the developer menu?
We can probably design a "Background Services" or "Running apps" menu to allow users to see which apps and services are running on their device. But that's IMHO out of the scope of this bug. What we want to add here is a way for developers to see which service workers are registered and to allow them to manage them (update, unregister).
Comment 12•10 years ago
|
||
Agree with Fernando: "service workers" is acceptable in the Developer menu, I think.
I also agree with Tim: we should explain that service workers might consume data and/or battery in the background. Let me know if UX can help with copy there (ni? me if so), but hopefully we have a string that's already being used for similar warnings (if such exist).
Flags: needinfo?(firefoxos-ux-bugzilla)
Comment 13•10 years ago
|
||
Ah, sorry I didn't realize this is in the developer menu.
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Comment 14•10 years ago
|
||
Comment on attachment 8596756 [details]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master
Thanks for working on this. Please check my comments in github.
Attachment #8596756 -
Flags: review?(arthur.chen)
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8596756 [details]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master
Thanks for the feedback folks!
Arthur, I just updated the PR with all your comments addressed, except for the one regarding the localization of "Service Workers". This string is not localized on Desktop or Android and I believe it shouldn't also be localized on b2g.
Attachment #8596756 -
Flags: review?(arthur.chen)
Assignee | ||
Updated•10 years ago
|
Component: General → Gaia::Settings
Comment 16•10 years ago
|
||
Comment on attachment 8596756 [details]
[gaia] ferjm:bug1155758.aboutserviceworkers > mozilla-b2g:master
r=me with the last comment addressed, thanks!
Attachment #8596756 -
Flags: review?(arthur.chen) → review+
Comment 17•10 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S12 (15may)
Assignee | ||
Comment 19•10 years ago
|
||
Comment 20•10 years ago
|
||
Hi,
just adding that while a SW is loaded being Settings->Developer->Service Worker open, the SW won't be shown in settings till closing and opening it again.
STR:
1- Go to Settings->Developer->Service Worker; no SW -> OK
2- Go to Browser and register a SW
3- Go to Settings->Developer->Service Worker; no SW -> I'd expect to see the SW details
4- Close Settings
5- Go to Settings->Developer->Service Worker; SW details are properly shown
Please let me know if a follow up should be open. Thanks!
Flags: needinfo?(ferjmoreno)
Comment 21•10 years ago
|
||
I forgot to mention that going back to Settings->Developer and accessing again from there to Service Worker is the same, the info is not refreshed, it is necessary to close and open Settings in order to see the refreshed information (loaded/unloaded... service workers). Thanks!
Blocks: 1196652
You need to log in
before you can comment on or make changes to this bug.
Description
•