1313567 - Accept an object as the "author" in manifest.json

Status: VERIFIED FIXED

(WebExtensions :: Compatibility, defect, P1)

Description

[Andrei Petcu]

Attachment: https_everywhere-2016.9.21-an+fx.xpi

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0
Build ID: 20161027030211

Steps to reproduce:

I'm trying to install the attached extension.


Actual results:

I tried to install the extension but I keep getting "This add-on could not be installed because it appears to be corrupt"


Expected results:

I should have been able to install it. It works fine in Firefox 49 but it fails in Nightly.
I was able to install this exact addon in the past in Nightly alsi. It has not changed, the only thing changed was Firefox Nightly. I tried with a new profile and it still fails. I even unpacked it and signed it with "web-ext sign".

Comment 1

[Andrei Petcu]

Here is the print screen I'm getting:
http://imgur.com/a/DzFVi

Comment 2

[Loic]

Reg range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2f033b5b7fd6e
aff5cc7a6e5209e52f0a77ee607&tochange=23887d112341a01d14e2a65b87b7a49601e17ed2

Andy McKay — bug 1282977 handle developer in manifest r=kmag

Andy, is this error expected after your patch in bug 1282977?

Comment 3

[Andy McKay]

The format of:

    "author": {
        "email": "[e-mail]"
    },

Has never been valid as far as I'm aware, it's not documented and doesn't work in the latest Firefox or Chrome. The only difference is in the past it failed silently, now it doesn't it fails with (on the command line):

1477672110725	addons.webextension.<unknown>	ERROR	Loading extension 'null': Reading manifest: Error processing author: Expected string instead of {"email":"[e-mail]"}

I think we should keep this open to make that error clearer to the add-on developer.

Comment 4

[Andy McKay]

I've found 19 add-ons in dxr that use this format, it might make sense to ignore the author property if its not a string.

Comment 5

[Kris Maglione [:kmag]]

:/ Chrome doesn't actually document what this is supposed to be, but it looks like they accept an object like this. Since this is going to break existing extensions, we should try to get it fixed before the next merge.

Comment 6

[Marcia Knous [:marcia]]

Tracking 52+ since this affects 19 extensions and is deemed a blocker bug.

Comment 7

[Andy McKay]

Attachment: bug 1313567 use author as fallback if its a string, otherwise ignore

Review commit: https://reviewboard.mozilla.org/r/89384/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/89384/

Comment 8

[Andrei Petcu]

Changed the version to v51 since DevEdition seems to be affected. There is another error message dough: "Firefox Developer Edition prevents this site from asking you to install software on your computer."

https://imgur.com/a/kFeJE

Stable (v49) and beta (v50) seem to be fine. I just tested them.

Comment 9

[Kris Maglione [:kmag]]

Comment on attachment 8805659 [details]
bug 1313567 use author as fallback if its a string, otherwise ignore

https://reviewboard.mozilla.org/r/89384/#review88972

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm:1007
(Diff revision 1)
>      // Use the raw manifest, here, since we need values with their
>      // localization placeholders still in place.
>      let rawManifest = extension.rawManifest;
>  
> -    let creator = rawManifest.author;
> +    // As a convenience, allow author to be set if its a string bug 1313567.
> +    let creator = typeof(rawManifest.author) === 'string' ? rawManifest.author : null;

If we're going to continue to accept this when it's a string, then we should keep the schema entry, but add `onError: "warn"`.

r=me with that change.

Comment 10

[Andy McKay]

Comment on attachment 8805659 [details]
bug 1313567 use author as fallback if its a string, otherwise ignore

https://reviewboard.mozilla.org/r/89384/#review88972

> If we're going to continue to accept this when it's a string, then we should keep the schema entry, but add `onError: "warn"`.
> 
> r=me with that change.

Ah that's cool, I somehow forgot about that, much nicer. Thanks.

Comment 11

[Andy McKay]

Comment on attachment 8805659 [details]
bug 1313567 use author as fallback if its a string, otherwise ignore

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/89384/diff/1-2/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf766b897b73
use author as fallback if its a string, otherwise ignore r=kmag

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bf766b897b73

Comment 14

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce the initial issue on Firefox 52.0a1 (2016-10-27) under Windows 10 64-bit.
Verified as fixed on Firefox 52.0a1 (2016-11-03) under Windows 10 64-bit and Ubuntu 16.04 32-bit. The webextension is successfully installed and the following warning appears in Browser Console:


1478185796773	addons.webextension.{37BC59A6-9C96-11E6-935F-F1E010FC8C42}	WARN	Loading extension '{37BC59A6-9C96-11E6-935F-F1E010FC8C42}': Reading manifest: Error processing author: Expected string instead of {"email":"[e-mail]"}