Open
Bug 1481012
Opened 6 years ago
Updated 1 year ago
JSONC support: When viewing manifest.json Firefox' JSON viewer should ignore // comments
Categories
(DevTools :: JSON Viewer, defect, P3)
Tracking
(Not tracked)
NEW
People
(Reporter: 5i13ghzt462u, 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
Comment 1•6 years ago
|
||
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
Comment 2•6 years ago
|
||
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
Comment 3•6 years ago
|
||
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 5•6 years ago
|
||
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)
Comment 7•6 years ago
|
||
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.
Updated•6 years ago
|
Priority: -- → P3
Comment 9•6 years ago
|
||
(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•5 years ago
|
||
Vote since I'd like to open files like https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/toolkit/components/extensions/schemas/web_request.json
Updated•2 years ago
|
Severity: normal → S3
Updated•2 years ago
|
Summary: When viewing manifest.json Firefox' JSON viewer should ignore // comments → JSONC Support
Updated•2 years ago
|
Summary: JSONC Support → JSONC support: When viewing manifest.json Firefox' JSON viewer should ignore // comments
You need to log in
before you can comment on or make changes to this bug.
Description
•