1481012 - JSONC support: When viewing manifest.json Firefox' JSON viewer should ignore // comments

Status: NEW

(DevTools :: JSON Viewer, defect, P3)

Description

[rugk]

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

Comment 1

[Kanchan Kumari QA]

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!

Comment 2

[Kris Maglione [:kmag]]

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.

Comment 3

[Oriol Brufau [:Oriol]]

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.

Comment 4

[Soledad Penades [:sole] [:spenades]]

Needinfoing Honza as he requested! :)

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

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

Comment 6

[rugk]

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.

Comment 7

[Oriol Brufau [:Oriol]]

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.

Comment 8

[rugk]

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.

Comment 9

[Oriol Brufau [:Oriol]]

(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.

Comment 10

[Junior [inactive]]

Vote since I'd like to open files like https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/toolkit/components/extensions/schemas/web_request.json