Open Bug 1481012 Opened 2 years ago Updated 2 months ago

When viewing manifest.json Firefox' JSON viewer should ignore // comments

Categories

(DevTools :: JSON Viewer, defect, P3)

62 Branch
defect

Tracking

(Not tracked)

People

(Reporter: c4609174, Unassigned)

Details

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0
Build ID: 20180804000531

Steps to reproduce:

Create a WebExtension with a manifest.json with a // comment.


Actual results:

When you load it and click on "Manifest URL" in about:debugging, the JSOn viewer of Firefox does show an error:

SyntaxError: JSON.parse: expected double-quoted property name at line 41 column 1 of the JSON data


Expected results:

It should not show an error, as in this case // comments are valid, and are even in the spec:

* https://discourse.mozilla.org/t/manifest-json-should-mention-it-allows-comments/27529
* it's even in the spec: https://browserext.github.io/browserext/#manifest-json
* https://developer.mozilla.org/en-US/Add-ons/WebExtensions/manifest.json
Summary: When viewing manifest.json Firefox viewer should ignore // comments → When viewing manifest.json Firefox' JSON viewer should ignore // comments
I was unable to create my first web extension :) getting error "There was an error during installation: JSON.parse: unexpected character at line 1 column 1 of the JSON data"

I couldn't test it. I am placing this under webextension:compatibility so someone can look into this. Thanks!
Component: Untriaged → Compatibility
Product: Firefox → WebExtensions
I suppose this might be worth handling, given that we link to manifest.json files from about:debugging, and JSON configuration files which support comments are becoming more common. But it's easier said than done, if we want to try to actually preserve the comments to display in the JSON viewer. And I suspect that we'd get at least as many complaints about silently dropping comments in the JSON viewer as we'd get for not supporting those files in the JSON viewer at all.
Component: Compatibility → JSON Viewer
Product: WebExtensions → DevTools
Clicking "Manifest URL" in about:debugging shows it as raw text for me, doesn't load the JSON Viewer.

If manifests allow comments then they are not pure JSON, maybe consider them a JSON5 subset or something. So I don't think they should be loaded with the JSON Viewer.
Needinfoing Honza as he requested! :)
Flags: needinfo?(odvarko)
I can't reproduce the problem either - clicking on the `Manifest URL` opens raw text for me, not JSON Viewer

Here is what I am doing:
1) Loading about:debugging
2) Clicking "Load Temporary Add-on" button
3) Picking a dir where my extension lives
4) When loaded, clicking the `Manifest URL`
New tab is opened and I see raw text of the manifest.json file

Testing in Nightly, Win10

@rugk: what am I doing wrong? How can I see the problem on my machine?

Honza
Flags: needinfo?(odvarko) → needinfo?(c4609174)
I also just reproduced it again with Firefox stable and some extensions.
I also saw, it does indeed display the raw text when you use extensions installed from AMO. So you have to load them manually.

I could reproduce it with:
* both my extensions https://github.com/rugk/offline-qr-code and https://github.com/rugk/how-did-i-get-here
* the "official" examples like https://github.com/mdn/webextensions-examples/tree/master/session-state and https://github.com/mdn/webextensions-examples/tree/master/tabs-tabs-tabs

Also, I have add-on debugging enabled, of course.

---

But I guess, I know why you cannot reproduce this. In Firefox on Linux WebExtensions did not run in it's own process, i.e. extensions.webextensions.remote was not enabled yet (ref bug 1357487).
As such, disable that and you can reproduce it.

However, I doubt it was intended that extensions.webextensions.remote disabled the JSON viewer. As such, I'll comment on bug 1357487.
Flags: needinfo?(c4609174)
I can confirm that the JSON Viewer is loaded for temporary extensions when extensions.webextensions.remote=false

In getMIMETypeFromContent I would prevent it from being loaded in the moz-extension:// scheme in order to be consistent. But it seems there is an underlying issue with content sniffers.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Actually, IMHO, I would really prefer the JSON viewer for WebExtension manifest.json etc. (You can e.g. reproduce it in the same way with the JSON _locales in WebExtension, which also support comments: e.g. /_locales/en/messages.json)

So if we could the JSON viewer _and_ have comments maybe just ignored in it (IMHO you do not need to display them.), this would be great.
(In reply to rugk from comment #8)
> Actually, IMHO, I would really prefer the JSON viewer for WebExtension
> manifest.json etc.

But loading the JSON Viewer for non-temporary extension or when when extensions.webextensions.remote=true is more difficult than disabling it in the opposite case. I prefer the latter for now in order to have consistency, because I don't think content sniffers will be fixed in a near future (see e.g. bug 1397966).

> So if we could the JSON viewer _and_ have comments maybe just ignored in it
> (IMHO you do not need to display them.), this would be great.

But comments are not valid JSON. WebExtension are using a JSON superset, so it's not really JSON.

Also, ignoring comments requires a custom JSON parser. I started writing one but of course performance was much worse than the native one for huge JSON.
You need to log in before you can comment on or make changes to this bug.