Closed Bug 1584518 Opened 5 months ago Closed 5 months ago

Misleading error message upon parsing error in manifest.json

Categories

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

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: eyalroz, Assigned: darktrojan)

Details

Attachments

(1 file)

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: loadManifestFromWebManifest@XPIInstall.jsm:426:17

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

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?

Type: defect → enhancement
Flags: needinfo?(geoff)

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

Flags: needinfo?(geoff)

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

Assignee: nobody → geoff
Status: NEW → ASSIGNED
Attachment #9097255 - Flags: review?(jorgk)

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"

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 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?
Attachment #9097255 - Flags: review?(jorgk) → review+

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

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?

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

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 71.0
You need to log in before you can comment on or make changes to this bug.