1224591 - Show developers more informative errors when an add-on can't be loaded

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dan Callahan [:callahad]]

STR:

(Example in the "plus_in_id" folder of https://github.com/callahad/webextension-tests/)

1. Set the "id" field in your add-on's manifest to "[e-mail]"
2. Package the add-on into an xpi
3. Attempt to install the add-on

What Should Happen:

- The add-on installs

What Actually Happens:

- Firefox complains that "this add-on could not be installed because it appears to be corrupt."

Replacing the "+" with "-" results in a working add-on.

This was reported by @Louis_Remi on Twitter: https://twitter.com/Louis_Remi/status/665184130032910336

:andym, do we need an additional bug for providing better diagnostic messages about failures, or do we expect stuff like this to just fall into place naturally with Bug 1185460?

Comment 1

[Andy McKay]

If the add-on fails to load, it would be nice to show the error nicely. Note that jpm will show an error and the browser console will show an error:

1447457738000	addons.xpi	WARN	Invalid XPI: Error: Illegal add-on ID the-id-is-something-that+magopian-swears-doesnt-exist

Exactly what the development flow is for an add-on is expected to be is a bit confusing to me at the moment.

Comment 2

[Kris Maglione [:kmag]]

Why does this matter?

Comment 3

[Kris Maglione [:kmag]]

(The + character in the ID, that is. The bad diagnostic messages should definitely be fixed.)

Comment 4

[Dan Callahan [:callahad]]

(In reply to Kris Maglione [:kmag] from comment #2)
> Why does this matter?

If IDs look enough like email addresses, people are going to naturally try to use sub-addressing. E.g., [e-mail], just like @Louis_Remi did above. Seems like the path of least resistance is to take a line from Burger King and say "have it your way."

Comment 5

[Kris Maglione [:kmag]]

I think I'd rather people learn that the IDs do not need to be actual email addresses, so they don't feel obligated to use them. A proper error message should take care of that.

In general, I don't really have any interest in emulating 4th rate fast food chains, so that sounds like a good counterargument to me.

Comment 6

[Andy McKay]

I agree with Kris, let's ensure the rules are simple, documented and enforced. I'd like to see a nice clear message and this case its a matter providing more information instead of just saying its corrupt. This bug belongs in the Add-on manager though.

Comment 7

[Tony Mechelynck [:tonymec]]

I recently noticed another instance of this confusing "Add-on appears to be corrupt" message: If an XPI has no install.rdf at top level, a doorhanger appears, saying "The addon from could not be installed because it appears to be corrupt" (with no location after "from" in the first case of it I found, which was in trying to reinstall ChatZilla from the cZ build distributed with SeaMonkey in its <appdir>/distribution/extensions directory). No indication of the particular kind of corruption (see bug 1223210 comment #16 sqq. and bug 1233969). The message is of course correct but not very informative, so my argument is analogous to comment #6 with a different kind of "corruption". Here too, a nice clear message would have been welcome.

Of course, tracking all possible cases of add-on corruption might be an endless toil, so maybe the original imprecise message should be kept as a fallback; but we have already identified two specific cases which IMHO deserve a more specific error message:
- The add-on ID contains invalid characters (or maybe: characters other than... with a regexp)
- No install.rdf was found in this add-on.

Another possible case has not yet happened to me, but I can think of it as a third type:
- This file was not recognized as in ZIP format (i.e. Firefox [or Thunderbird or SeaMonkey etc.] did not succeed to unzip it).

Comment 9

[Tim Nguyen :ntim]

I've been having these errors that chrome caught, but not firefox (firefox only showed: extension is invalid):
- icon could not be loaded
- default_locale was specified but not locales

I

Comment 11

[Rodney Folz]

I believe I'm having a similar issue.

When developing, I tried to reload my extension in about:debugging. The load failed and the only error message I got was "There was an error during installation: Extension is invalid".

I ran web-ext lint on my codebase and it produced no errors.

On a hunch, I checked the Browser Console and there was a perfectly helpful error message: "1479348916445	addons.webextension.<unknown>	ERROR	Loading extension 'null': Reading manifest: Error processing background: Error processing background: Property "scripts" is required". (I had accidentally deleted that line.)

I would have loved to see that error message inline when I tried to reload my extension the first time.

Comment 12

[Andrew Swan [:aswan]]

It looks like temporary installation from about:debugging didn't exist at all when this was first filed, plus we've made a bunch of incremental improvements to error reporting there.  For general bugs we have things like bug 1226743.