1584518 - Misleading error message upon parsing error in manifest.json

Status: RESOLVED FIXED

(Thunderbird :: Add-Ons: Extensions API, enhancement)

Description

[Eyal Rozenberg]

If you try to install an extension in Thunderbird 68, and there's a syntax error in the extension's manifest.json - such as an extra comma, or a duplicate field - you'll get a message saying that the extension is "incompatible with this version of Thunderbird" - which is misleading. It would be "incompatible" with other versions as well - the author just needs to fix the syntax.

Looking at the error console, we see entries such as:

1569587050843 addons.xpi WARN Invalid XPI: Error: Extension is invalid(resource://gre/modules/addons/XPIInstall.jsm:426:17) JS Stack trace: [e-mail]:426:17

there should be a distinction between the exception thrown on version incompatibility and on simple failure to parse.

Comment 1

[Jorg K (CEST = GMT+2)]

I've been tearing my hair out over his. However, I recently found online JSON validators, like here: https://jsonformatter.curiousconcept.com/

Geoff, can that be improved?

Comment 2

[Geoff Lankow (:darktrojan)]

We used the current message because a broken manifest.json is indistinguishable from no manifest.json (that is, an install.rdf extension) and that was a much more likely scenario. On Daily I think it's time we go back to the usual message, which is "This add-on could not be installed because it appears to be corrupt."

Comment 3

[Geoff Lankow (:darktrojan)]

Attachment: 1584518-remove-legacy-message-1.diff

Jorg, you reviewed this code going in, you can review it coming out.

Comment 4

[Eyal Rozenberg]

a broken manifest.json is indistinguishable from no manifest.json

Umm, that's prima facie untrue. It is very distinguishable, and the distinction is quite important - especially to extension developers.

"This add-on could not be installed because it appears to be corrupt."

That is really not good enough. The file whose parsing failed should be mentioned, e.g.

"This add-on could not be installed because its manifest file appears to be corrupt"

or

"This add-on could not be installed because its manifest.json file appears to be corrupt"

Comment 5

[Geoff Lankow (:darktrojan)]

It is true, at the point where the message shown is chosen. There's an error code of AddonManager.ERROR_CORRUPT_FILE (-3) which could mean all sorts of things. If you want a better message you'd better talk to the Mozilla add-ons team.

Comment 6

[Jorg K (CEST = GMT+2)]

Comment on attachment 9097255 [details] [diff] [review]
1584518-remove-legacy-message-1.diff

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

::: mail/base/modules/ExtensionsUI.jsm
@@ +618,5 @@
>                : "addonLocalInstallError";
>            let args;
>  
>            // Temporarily replace the usual warning message with this more-likely one.
> +          if (install.error != 0) {

Hmm, the original was < 0:
https://hg.mozilla.org/comm-central/rev/52e953facdcf#l1.12
What's the difference between > 0 and < 0?

Comment 7

[Jorg K (CEST = GMT+2)]

Clarified via IRC:
09:32:11 - darktrojan: change the != 0 to < 0 ...

Comment 8

[Eyal Rozenberg]

If you want a better message you'd better talk to the Mozilla add-ons team.

Well, I do, and that's why I filed this bug. While it's true that replacing the misleading error message with a vague one is an improvement, it's not satisfactory IMHO... should I perhaps add someone else to the CC list?

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/538d73551084
Remove "incompatible" message when installing corrupt or legacy add-on. r=jorgk DONTBUILD