Error about duplicate properties in manifest.json

[kumar303]

The linter should show a warning (or error?) if a manifest has duplicate properties in it. This will lead to confusing results when the JSON is parsed by Firefox (I think the last one wins). Example:

{
  "name": "My Extension",
  "name": "My Extension 2",
  "version": "1.0"
}

[tofumatt]

Oh, good call. I think a straight up error is appropriate here.

• tofumatt

On 30 June 2016 at 22:43:25, Kumar McMillan ([email protected])
wrote:

The linter should show a warning (or error?) if a manifest has duplicate
properties in it. This will lead to confusing results when the JSON is
parsed by Firefox (I think the last one wins). Example:

{
"name": "My Extension",
"name": "My Extension 2",
"version": "1.0"
}

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#803, or mute the thread
https://github.com/notifications/unsubscribe/AAFi923pOkEnOx4BuDNm5vVnzswDibnTks5qRDh9gaJpZM4JCnxn
.

[tofumatt]

Doing some research it seems this is totally allowed by the JSON spec (though not recommended, it is technically allowed) and most parsers (PHP, node, Python) just replace the value of the key whenever it hits a new one.

That said, we can check for duplicate keys pretty easily with JSON.parse's reviver parameter, so I'll get this done today! 😄

[ValentinaPC]

I was able to upload WebExtensions with duplicate keys in manifest, without any issues:

• a WebExtension with 2 "name" keys (the name of the WebExtension after submissions was the second)
• a WebExtension with 2 "versions" keys (the version of the WebExtension after submission was the second aka the last one)

Verified using FF47 and FF50 (Win 7).
Screenshots:

[muffinresearch]

@ValentinaPC This might not be released yet. When a patch lands in addons-linter it takes a package release and a lib bump in addons-server to be at a point ready to test.

[ValentinaPC]

I saw this closed... so... I've tested! 😄

[muffinresearch]

@ValentinaPC it's worth noting there'll always be a delay between closure and it being possible to test for every single addons-linter PR. We need to figure a better way to communicate that.

@tofumatt got any thoughts on how we can make it clearer for addons-linter that there's a dependency on an npm release and a corresponding package bump in addons-server? Maybe we need an unreleased label or similar? Bonus points if it's automated and changes automatically when addons-servers package.json contains a version with a given patch.

[tofumatt]

Unreleased or similar works for me. Automating that sounds neat but as we work less on linter might not be worth the effort unless there’s prior art 😃

[andymckay]

This works for me. Closing, if you want to do something about notifications about when we push, please make that a seperate bug.