The default bug view has changed. See this FAQ.
Bug 997779 (webmanifest)

[Meta] Implement Web Manifest spec in Gecko

NEW
Assigned to

Status

()

Core
DOM
3 years ago
13 days ago

People

(Reporter: overholt, Assigned: marcosc)

Tracking

(Depends on: 5 bugs, Blocks: 6 bugs, {dev-doc-needed})

Trunk
dev-doc-needed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
http://w3c.github.io/manifest/
Discussing this with Marcos in person, I suggested that we handle the mozbrowsermanifestchange event on the parent side, do the processing of the manifest, and then expose the results through the mozbrowser API.  This way we can test the processing code in Gecko, and then the browser app in Gaia can use the API to get the manifest information from the web pages.

Does this sound sane?
Keywords: dev-doc-needed
Sounds fine to me, but what exactly will we expose to gaia? A sanitized manifest? something else?
(Assignee)

Comment 3

3 years ago
(In reply to Fabrice Desré [:fabrice] from comment #2)
> Sounds fine to me, but what exactly will we expose to gaia? A sanitized
> manifest? something else?

I guess it will just be a sanitized manifest (just hand you an Object somehow). However, if you require something more fancy/useful, we can come up with something better.
(Assignee)

Comment 4

3 years ago
Created attachment 8432815 [details] [diff] [review]
manifest.patch

This is by no means done, just putting this up for initial review - just to get a sense of direction and feedback. 

I'm a workshop all this week so I figured I'd start gathering feedback. Remember I'm new to Gecko and don't know the internal classes - so if there is better stuff I should be using (e.g., I know the security check stuff is wrong), then let me know what I should be using. 

The code starts at ManifestObtainer.js. The ManifestObtainer relies on ManifestProcessor to process the manifest. The ManifestProcessor makes use of the IconsProcessor.js.
Attachment #8432815 - Flags: review?(bfrancis)
Comment on attachment 8432815 [details] [diff] [review]
manifest.patch

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

Hi Marcos, I'm not the right person to review this patch, maybe Fabrice would like to take a look.

One question, what will be the difference between the manifest data Gecko retrieves and the manifest data that gets passed on to Gaia?
Attachment #8432815 - Flags: review?(bfrancis) → review?(fabrice)
(Assignee)

Comment 6

3 years ago
(In reply to Ben Francis [:benfrancis] from comment #5) 
> Hi Marcos, I'm not the right person to review this patch, maybe Fabrice
> would like to take a look.

No probs. 

> One question, what will be the difference between the manifest data Gecko
> retrieves and the manifest data that gets passed on to Gaia?

The data Gaia gets is a cleaned up manifest that includes defaults. Like:

{
  "name": "   test   ",
  "foo": "bar"
} 

Would return something like this to Gaia:

{
   "name": "test", 
   "icons": [], 
   "display": "browser"
}
(Assignee)

Updated

3 years ago
Flags: needinfo?(fabrice)
(Assignee)

Comment 7

3 years ago
Fabrice, now that you are back. Can you take a look at the general direction of the attached patch?
(Assignee)

Comment 8

3 years ago
I'll keep hacking at it and see if I can find someone else to review it.
Flags: needinfo?(fabrice)
Comment on attachment 8432815 [details] [diff] [review]
manifest.patch

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

Sorry for taking so long - I did a first pass.

The overall design looks fine, with the exception of the error reporting from process().
From a coding style pov, please switch to 2 spaces per indent. We also usually use double quotes in chrome code for strings. There's a bunch of minor whitespace issues too (eg check all the |if () {| blocks)

::: dom/manifest/IconsProcessor.jsm
@@ +4,5 @@
> +/* exported IconsProcessor */
> +'use strict';
> +this.EXPORTED_SYMBOLS = ['IconsProcessor'];
> +
> +var extractValue;

s/var/let

@@ +5,5 @@
> +'use strict';
> +this.EXPORTED_SYMBOLS = ['IconsProcessor'];
> +
> +var extractValue;
> +function IconsProcessor(valueExtractor) {

you need this.IconsProcessor = function(...) to export it properly.

@@ +101,5 @@
> +                sizes = this.processSizesMember(potentialIcon),
> +                icon = {
> +                    'src': src,
> +                    'type': type,
> +                    'sizes': sizes

nit: no need for quotes around the property names.
you can also just write:
icons.append({
  src: this.process...
  type: this.process...
  sizes: this.process...
});

::: dom/manifest/ManifestObtainer.jsm
@@ +21,5 @@
> +Cu.importGlobalProperties(['XMLHttpRequest', 'URL']);
> +Cu.import('resource://gre/modules/XPCOMUtils.jsm');
> +XPCOMUtils.importRelative(this, 'ManifestProcessor.jsm');
> +
> +function ManifestObtainer() {

you need this.ManifestObtainer = function() to export it properly.

@@ +24,5 @@
> +
> +function ManifestObtainer() {
> +    const wellKnownURI = '/.well-known/manifest.json';
> +    const manifestQuery = 'link[rel~="manifest"]';
> +    const manifestProcessor = new ManifestProcessor();

why not create it in the only place you use it?

@@ +60,5 @@
> +
> +    function fetchManifest(manifestURL, contentWindow, useCreds) {
> +        return new Promise((resolve, reject) => {
> +            var xhr = new contentWindow.XMLHttpRequest();
> +            xhr.withCredentials = useCreds; 

You also likely want xhr.mozBackgroundRequest = true;

@@ +64,5 @@
> +            xhr.withCredentials = useCreds; 
> +            xhr.open('GET', manifestURL);
> +            xhr.onload = () => {
> +                let promiseResult = {
> +                    text: xhr.responseText,

Since you are only interested in json manifests, I would just get set the xhr.responseType to 'json' and s/text/manifest : xhr.response

@@ +68,5 @@
> +                    text: xhr.responseText,
> +                    manifestURL: manifestURL,
> +                    docLocation: new URL(contentWindow.location)
> +                };
> +                if (xhr.status.toString().startsWith('2')) {

we usually rather do >=200 && < 300

@@ +86,5 @@
> +    }
> +
> +    function processManifest(obj) {
> +        return new Promise((resolve) => {
> +            let manifest = manifestProcessor.process(obj.text, obj.manifestURL, obj.docLocation);

why not just pass obj to process() ?

@@ +87,5 @@
> +
> +    function processManifest(obj) {
> +        return new Promise((resolve) => {
> +            let manifest = manifestProcessor.process(obj.text, obj.manifestURL, obj.docLocation);
> +            resolve(manifest);

the error management around process() is unclear

::: dom/manifest/ManifestProcessor.jsm
@@ +55,5 @@
> +    processedManifest.start_url = this.processStartURLMember(manifest, manifestURL, docURL);
> +    processedManifest.display_mode = this.processDisplayMember(manifest);
> +    processedManifest.orientation = this.processOrientationMember(manifest);
> +    processedManifest.name = this.processNameMember(manifest);
> +    processedManifest.icons = this.processIconsMember(manifest, manifestURL);

in other places you do:
processedManifest = {
  start_url: ...
  ...
}

@@ +129,5 @@
> +    targetURI =  toNsIURI(docURL);
> +    referrerURI = toNsIURI(potentialResult);
> +    sameOrigin = sm.checkSameOriginURI(referrerURI, targetURI, false);
> +    if(!sameOrigin){
> +        issueDeveloperWarning();

is that just a warning, not an error?
Attachment #8432815 - Flags: review?(fabrice)
(Assignee)

Comment 10

3 years ago
Thanks Fabrice for the feedback. I'm about to go on vacation for a couple of weeks but will make the changes as soon as I'm back.
Blocks: 1003890
Hi Marcos, just wondered about the progress on this as I'm starting to play with the Gaia side.

Should I just polyfill this for the time being and do the XHR content side, or is this likely to land some time soon?

Thanks
Flags: needinfo?(mcaceres)
(Assignee)

Comment 12

3 years ago
I'm happy for you to handle the integration into Gaia. I already did most of the work on the processing side, and I'm happy to collaborate on that.
Flags: needinfo?(mcaceres)
(Assignee)

Comment 13

3 years ago
Restarting work on this.
(Assignee)

Updated

3 years ago
Depends on: 1077620
(Assignee)

Updated

3 years ago
Depends on: 1079453
(Assignee)

Updated

3 years ago
Depends on: 1083410
(Assignee)

Updated

2 years ago
Depends on: 1089255
Blocks: 1097533
(Assignee)

Updated

2 years ago
Depends on: 1104916
(Assignee)

Updated

2 years ago
Summary: Implement W3C Manifest spec → [Meta] Implement Web Manifest spec
(Assignee)

Updated

2 years ago
(Assignee)

Updated

2 years ago
Depends on: 1119670
(Assignee)

Updated

2 years ago
Alias: webmanifest
Summary: [Meta] Implement Web Manifest spec → [Meta] Implement Web Manifest spec in Gecko
(Assignee)

Updated

2 years ago
No longer depends on: 1089255
(Assignee)

Updated

2 years ago
Blocks: 1143879
(Assignee)

Updated

2 years ago
No longer blocks: 1143879
Depends on: 1143879
(Assignee)

Updated

2 years ago
Depends on: 1143898
(Assignee)

Updated

2 years ago
Depends on: 1086997
(Assignee)

Updated

2 years ago
Depends on: 1153958
(Assignee)

Updated

2 years ago
No longer depends on: 1153958
(Assignee)

Updated

2 years ago
Depends on: 1162729
(Assignee)

Updated

2 years ago
Depends on: 1162808
(Assignee)

Updated

2 years ago
Depends on: 1164235

Comment 14

2 years ago
After done, don't forget to update https://developer.mozilla.org/en-US/Apps/Build/Manifest documentation.
(Assignee)

Comment 15

2 years ago
(In reply to Binyamin from comment #14)
> After done, don't forget to update
> https://developer.mozilla.org/en-US/Apps/Build/Manifest documentation.

That's a separate (Mozilla proprietary) format, which is not compatible with this standardized format. 

We will need to create a new set of documents for this format.
(Assignee)

Updated

2 years ago
Depends on: 1166405
(Assignee)

Updated

2 years ago
Depends on: 1167335
(Assignee)

Updated

2 years ago
Depends on: 1167806
(Assignee)

Updated

2 years ago
Blocks: 1168465
(Assignee)

Updated

2 years ago
Depends on: 1168877
(Assignee)

Updated

2 years ago
Depends on: 1089255
(Assignee)

Updated

2 years ago
Depends on: 1169784
(Assignee)

Updated

2 years ago
Depends on: 1171200
(Assignee)

Updated

2 years ago
Depends on: 1176442
(Assignee)

Updated

2 years ago
Depends on: 1178175
(Assignee)

Updated

2 years ago
Depends on: 1178655
(Assignee)

Updated

2 years ago
Blocks: 1179083
(Assignee)

Updated

2 years ago
Depends on: 1180652
Blocks: 1186134
(Assignee)

Updated

2 years ago
Blocks: 1187027
(Assignee)

Updated

2 years ago
No longer blocks: 1187027
(Assignee)

Updated

2 years ago
Depends on: 1186908
(Assignee)

Updated

2 years ago
Depends on: 1195018

Comment 16

2 years ago
(In reply to Marcos Caceres [:marcosc] from comment #15)
> (In reply to Binyamin from comment #14)
> > After done, don't forget to update
> > https://developer.mozilla.org/en-US/Apps/Build/Manifest documentation.
> 
> That's a separate (Mozilla proprietary) format, which is not compatible with
> this standardized format. 
> 
> We will need to create a new set of documents for this format.

The current Docs also point to the (new) Manifest spec. This is now totally confusing. These MDN pages need to be revamped and warnings added that the feature is not to be confused with the W3C manifest spec. Though I don't know how. Best would probably be renaming the pages and links from/to this site to something like "Open Web Apps Manifest" and add a big fat warning box on this page and the subordinate pages. 

It's also unclear how big the difference between the (old) manifests and the W3C manifests are – another section explaining the different backgrounds and implementations would be nice even if (and especially when) the MDN pages for the W3C manifests are not written yet. Google claims[1] that Firefox implements a proprietary API. We should clear that up.

Can you guys please provide some input (or get to the action)?

[1] https://www.chromestatus.com/feature/6488656873259008
Flags: needinfo?(sebastianzartner)
Flags: needinfo?(jypenator)

Comment 17

2 years ago
(In reply to Florian Bender from comment #16)
> (In reply to Marcos Caceres [:marcosc] from comment #15)
> > (In reply to Binyamin from comment #14)
> > > After done, don't forget to update
> > > https://developer.mozilla.org/en-US/Apps/Build/Manifest documentation.
> > 
> > That's a separate (Mozilla proprietary) format, which is not compatible with
> > this standardized format. 
> > 
> > We will need to create a new set of documents for this format.
> 
> The current Docs also point to the (new) Manifest spec. This is now totally
> confusing. These MDN pages need to be revamped and warnings added that the
> feature is not to be confused with the W3C manifest spec. Though I don't
> know how. Best would probably be renaming the pages and links from/to this
> site to something like "Open Web Apps Manifest" and add a big fat warning
> box on this page and the subordinate pages. 

We should start by stopping the "Open Web App" nonsense: it's deceptive and disingenuous at best - and just and outright lie at worst. It makes us look stupid and it's just embarrassing to anyone involved with actually standardizing true open web stuff through the W3C or WHATWG.

Let's please start calling the other format "Firefox OS's Manifest Format", as that is what it is. 

> It's also unclear how big the difference between the (old) manifests and the
> W3C manifests are

Apples and oranges: the only commonality is the use of JSON. 

>  – another section explaining the different backgrounds and
> implementations would be nice even if (and especially when) the MDN pages
> for the W3C manifests are not written yet. Google claims[1] that Firefox
> implements a proprietary API. We should clear that up.

Well, we don't currently support the w3c format in any shipping products. 

> Can you guys please provide some input (or get to the action)?
> 
> [1] https://www.chromestatus.com/feature/6488656873259008
(In reply to marcos from comment #17)

> We should start by stopping the "Open Web App" nonsense: it's deceptive and
> disingenuous at best - and just and outright lie at worst. It makes us look
> stupid and it's just embarrassing to anyone involved with actually
> standardizing true open web stuff through the W3C or WHATWG.

Please step down from your horse for a bit, while you update all the references to "Open Web App" to your liking. This is an historical naming, there's nothing more to read into it.

> Let's please start calling the other format "Firefox OS's Manifest Format",
> as that is what it is. 

Since you're unlikely to reuse "Open Web App manifest" for the new "Web Manifest" that seems totally useless. Can we stop wasting energy on that kind of things?

Comment 19

2 years ago
notechnicalvalue
```
                   ,%%%,
                 ,%%%` %==--
                ,%%`( '|
               ,%%@ /\_/
     ,%.-"""--%%% "@@__  
    %%/             |__`\       
   .%'\     |   \   /  //
   ,%' >   .'----\ |  [/
      < <<`       ||      
       `\\\       ||        
         )\\      )\
 ^^^^^^^^"""^^^^^^""^^^^^^^^
```
Ok, let's start by checking if I've understood things right :-)

1. Firefox OS apps have a proprietary manifest format, that *was* documented on the page [1].
2. The latest revision of this page with the proprietary manifest is [2].
3. W3C and browser vendors are developing and implementing a new manifest format. That's the purpose of this bug and the URL field has the link to it.
4. Over the months, since revision [2], the Firefox OS manifest document on MDN has been morphed to describe the W3C manifest.
5. The Firefox OS manifest will stay (or there is no concrete plan to get rid of it now).
6. Both manifests are incompatible (comment 15) and have different use cases.

Are these assertions correct? (If so, we will be able to move to a plan to clean things :-) )

[1] https://developer.mozilla.org/en-US/Apps/Build/Manifest 
[2] https://developer.mozilla.org/en-US/Apps/Build/Manifest$revision/800879
Flags: needinfo?(sebastianzartner)
Flags: needinfo?(mcaceres)
Flags: needinfo?(jypenator)

Comment 21

2 years ago
(In reply to Jean-Yves Perrier [:teoli] from comment #20)
> Ok, let's start by checking if I've understood things right :-)
> 
> 1. Firefox OS apps have a proprietary manifest format, that *was* documented
> on the page [1].

Correct.

> 2. The latest revision of this page with the proprietary manifest is [2].

Correct. 

> 3. W3C and browser vendors are developing and implementing a new manifest
> format. That's the purpose of this bug and the URL field has the link to it.

Correct.

> 4. Over the months, since revision [2], the Firefox OS manifest document on
> MDN has been morphed to describe the W3C manifest.

I gave it a quick scan, but can't see any overlap so hopefully incorrect.  

> 5. The Firefox OS manifest will stay (or there is no concrete plan to get
> rid of it now).

Correct.

> 6. Both manifests are incompatible (comment 15) and have different use cases.

Correct. 

> Are these assertions correct? (If so, we will be able to move to a plan to
> clean things :-) )

Seems good. Like I said above, it would be helpful if we made it more clear that this is a Mozilla proprietary format. 

> [1] https://developer.mozilla.org/en-US/Apps/Build/Manifest 
> [2] https://developer.mozilla.org/en-US/Apps/Build/Manifest$revision/800879
(Assignee)

Updated

2 years ago
Flags: needinfo?(mcaceres)
Ok, so it looks less grim that I originally thought. I've opening a doc bug to specifically address the cleaning of the page: bug 1208966
Depends on: 1237021

Updated

a year ago
Depends on: 1239358
Blocks: 1239358
No longer depends on: 1239358
Depends on: 1240490
Depends on: 1240735
See Also: → bug 1246312
See Also: bug 1246312
Blocks: 1201717
(Assignee)

Updated

a year ago
Depends on: 1250048
(Assignee)

Updated

a year ago
Blocks: 1212648
(Assignee)

Updated

a year ago
Depends on: 1258899
(Assignee)

Updated

a year ago
Depends on: 1262739
(Assignee)

Updated

a year ago
Depends on: 1264813
(Assignee)

Updated

a year ago
Depends on: 1264816
(Assignee)

Updated

a year ago
Depends on: 1264819
(Assignee)

Updated

11 months ago
Depends on: 1265279
(Assignee)

Updated

11 months ago
Depends on: 1266627
Depends on: 1266660
No longer depends on: 1266660
(Assignee)

Updated

10 months ago
Depends on: 1275160
(Assignee)

Updated

8 months ago
Blocks: 1285858
(Assignee)

Updated

6 months ago
Depends on: 1309099
No longer blocks: 1285858
You need to log in before you can comment on or make changes to this bug.