1551831 - legacy flag not working in Thunderbird 60

Status: RESOLVED WONTFIX

(Thunderbird :: Add-Ons: General, defect)

Description

[Axel Grude [:realRaven]]

Attachment: quickFilters 3.12.3 prerelease 33

When trying to port my Add-on quickFilters to make a compatible version that works for Tb60 and Tb68, the installation seems to fail in Tb 60.6.1 (32bit, Windows).

There is no UI and no entry in Tools - Addon Options. On installation, there is no prompt to restart (there should be) - which leads me to the conclusion that the installation is not complete.

I debugged this down to the level of Extension.jsm and indeed, the "legacy" flag seems to be unknown:

1557916668367 addons.webextension.<unknown> WARN Loading extension 'null': Reading manifest: Error processing legacy: An unexpected property was found in the WebExtension manifest.
1557916751577 [e-mail] WARN Loading extension '[e-mail]': Reading manifest: Error processing legacy: An unexpected property was found in the WebExtension manifest.

The version installs fine on Tb67 and Tb68.

I tried starting the profile with -purgecaches but had no success either. I tried installing on 2 different profiles (on the last one I was prompted to restart but still no UI / options entry)

I also debugged down to Extensions.jsm level and can confirm that "Legacy" is not a regular property - the list I found while debugging was:
applications, author, background, browser_specific_Settings, cloud_file, content_scripts, content_security_policy, default_locale, description, developer, experiment_apis, homepage_url, icons, incognito, manifest_version, minimum_chrome_version, minimum_opera_version, name, optional_permissions, options_ui, permissions, protocol_handlers, short_name, version, web_accessible_resources. (see https://i.imgur.com/gEfMYlG.png)

My own manifest has these entries: https://i.imgur.com/Z1LzUdJ.png
author, name, description, version, developer, applications, icons and legacy

Since the Add-on submission process blocks uploading "real legacy" Add-ons once a web extension is uploaded this will effectively block me from delivering anything for modern versions of Thunderbird until Tb60 is end-of-life. My workaround was to remove the WebExtension version for now. I am attaching a prerelease of quickFilters - it should at least add a toolbar button automatically.

Comment 1

[Andrei Hajdukewycz [:sancus]]

@darktrojan Is it possible for us to uplift patches to 60 so that people can build add-ons that will install in both 60 and 68 using a manifest.json?

Comment 2

[John Bieling (:TbSync)]

I just want to point out that the initial issue described in this bug can be solved by adding both, an install.rdf and a manifest.json.

As Geoff has put it: That is possible but not suggested. It can break at any time, and it will probably break when support for install.rdf is dropped on ATN after TB60 is end of life.

Nevertheless, the second issue described in this bug is a show stopper for the officially suggested method: If add-on authors modify their add-on to work with TB68 (without supporting TB60 and older) and upload it as a webextension to ATN, it will not be possible to update the TB60 version of the add-on anymore as ATN will not allow uploads of legacy add-ons, once a webextension has been uploaded.

This is bad because:

• I will not be able to provide an add-on version for Thunderbird Beta, as I do not want to risk to not being able to fix bugs in the add-on version for Thunderbird ESR (TB60).
• Once TB68 is ESR it will take a while until most users have switched, during that time I am not able to fix bugs in the add-on version for TB60

Again: This is only relevant for legacy extensions with install.rdf compatible up to TB60, which have a webextension successor not compatible with TB60 anymore.

There has been a lengthy discussion on IRC on how to fix this and since i have a bias, I cannot summarize all of it, but just my own suggestions:

1. Go around the ATN limitation by uploading add-ons in branched versioning. We tested this with the CategoryManager Add-On:
   https://addons.thunderbird.net/addon/categorymanager/versions/

As you can see, the TB60 version is using 2.* version numbers and the TB68 version is using 3.* version numbers. After 3.1 has landed, I was able to push 2.11 to ATN without being stopped during the upload procedure.

At this stage TB60 updated the add-on nicely from 2.09 to 2.11 and when installed anew using the AddOnManger, it found and installed 2.11. Thunderbird Daily (TB68) found and installed 3.1 as desired.

I then pushed 3.2 to ATN and manually installed 2.09 in TB60 and it was again updated to 2.11. Thunderbird Daily did not update to 3.2, because there is an issue with the update in daily in general. After that is fixed I will test that as well.

The only caveat: Uploading add-ons with lower version numbers is currently restricted by add-on review policy. The reason given is some potential issue with mirror propagation. Is that a real issue or can we allow that until the end of the year?

2. Allow to add a dummy manifest.json, which is completly ignored by TB60 but makes ATN think the add-on is actually a webext, so addon authors can release new versions for both (inc the version number each time as enforced by the review policy). Could there be an issue with ATN? I did not test this.

Comment 3

[Geoff Lankow (:darktrojan)]

(In reply to John Bieling from comment #2)

Nevertheless, the second issue described in this bug is a show stopper for the officially suggested method: If add-on authors modify their add-on to work with TB68 (without supporting TB60 and older) and upload it as a webextension to ATN, it will not be possible to update the TB60 version of the add-on anymore as ATN will not allow uploads of legacy add-ons, once a webextension has been uploaded.

You say this, but then contradict it.

The only caveat: Uploading add-ons with lower version numbers is currently restricted by add-on review policy. The reason given is some potential issue with mirror propagation. Is that a real issue or can we allow that until the end of the year?

This is news to me. Andrei, is this a real problem?

But anyway, back to the original question. Axel, you should include install.rdf if you want compatibility with 60. 60 will read it and ignore the manifest.json. 68 knows nothing about install.rdf, so ignores it.

Comment 4

[Andrei Hajdukewycz [:sancus]]

(In reply to Geoff Lankow (:darktrojan) from comment #3)

This is news to me. Andrei, is this a real problem?

So here is the issue. People want to upload addons for 60 and 68 simultaneously, which makes sense, since 60 is supported until at least Oct/Nov even after we release 68. There are 3 solutions proposed:

1. Make addons that have install.rdf and manifest.json in the same package. This is specifically recommended against in our documentation. I don't know why. I don't really have a problem with it, with the caveat that I am absolutely against making ATN read any sort of data from both manifests simultaneously. Do not want that.

2. Upload an addon with install.rdf for 60, and a separate addon with manifest.json for 68. These use different versioning schemes, such as 2.x for the old install.rdf and 3.x for the new manifest.json. So, a dev could upload 3.1, then 2.11, then 3.2, and so on.

This is recommended against in some AMO review policy that I can't find. I don't know what mirror propagation means, but maybe someone can link me to that policy? Fallen advised that this is unsupported and that there may be gotchas and unexpected from behaviour if you do this. It's impossible to know if there are hidden bugs with this approach because there is a lot of undefined behaviour in addons-server code in general.

3. Fallen suggested that we backport patches to 60 so that 60 can use manifest.json with "legacy" object as an alternative to either of these approaches. So that is why I asked the above question. I don't personally know or understand the difficulties involved in doing this.

(In reply to Geoff Lankow (:darktrojan) from comment #3)

68 knows nothing about install.rdf, so ignores it.

4. Allegedly 68 will still install bootstrapped addons that have an install.rdf, so I guess this isn't completely true? If that is the case, I really dislike the behaviour and I'd prefer 68 only supported addons via manifest.json. I would like to disable support for install.rdf legacy extensions in 2020 after 60 is unsupported, to pull in more upstream work. Having to support install.rdf-only uploads until after 76 is released would make this impossible.

Comment 5

[Geoff Lankow (:darktrojan)]

True, I had forgotten about bootstrapped add-ons using install.rdf, which is a different can of worms. I'll amend that statement: 68 ignores install.rdf if there is a manifest.json.

Comment 6

[John Bieling (:TbSync)]

(In reply to Geoff Lankow (:darktrojan) from comment #3)

(In reply to John Bieling from comment #2)

Nevertheless, the second issue described in this bug is a show stopper for the officially suggested method: If add-on authors modify their add-on to work with TB68 (without supporting TB60 and older) and upload it as a webextension to ATN, it will not be possible to update the TB60 version of the add-on anymore as ATN will not allow uploads of legacy add-ons, once a webextension has been uploaded.

You say this, but then contradict it.

Sorry for not being to the point. If you use the default approach and inc the version number of your add-on for each version uploaded to ATN, it will not allow to upload a legacy add-on after a webextension has been pushed.

You can get around this by slipping in legacy add-ons below the first webextension, by using smaller version numbers. But this will currently be declined during review. Erosman rejected categorymanager 2.10 with that reason and saying something about issues with mirror propagation. CategoryManger 2.11 was accepted as a test and it looks like there are no issues with ATN and updated and so on. Still need to do some tests with daily, which is not updating addons at all at the moment.

But maybe just adding an dummy manifest.json to legacy add-ons will also fix this (my second suggestion). Where does ATN reads its info from, when both files are prensent? Sancus: Should I go ahead an try this or are you not in favour of that at all?

Comment 7

[Axel Grude [:realRaven]]

(In reply to Geoff Lankow (:darktrojan) from comment #5)

True, I had forgotten about bootstrapped add-ons using install.rdf, which is a different can of worms. I'll amend that statement: 68 ignores install.rdf if there is a manifest.json.

And this is a good thing - because it currently allows for "hybrid" Add-on files that can be installed both by [current] Thunderbird 60.6.1 (which ignores manifest.json) and Tb 68 daily (which ignores install.rdf).

Any other incompatibilities can be worked around by using shimming methods in chrome.manifest, for example through content:

content shimQuickFilters chrome://quickfilters/content/shimEcmaOld/ application={3550f703-e582-4d05-9a08-453d09bdfdc6}
content shimQuickFilters chrome://quickfilters/content/shimEcma/ application={3550f703-e582-4d05-9a08-453d09bdfdc6} platformversion>=47

then refer to platform-specific shim code in your js:

<script type="application/javascript" src="chrome://shimQuickFilters/content/qFilters-shim-ecma.js" />

or if you import a js module:

resource quickfoldersShim60 chrome/content/shim60/ platformversion<61
resource quickfoldersShim60 chrome/content/shim61/ platformversion>=61

Components.utils.import("resource://quickfoldersShim60/quickfolders-util2.jsm");

Comment 8

[John Bieling (:TbSync)]

I did some more tests today and I might have found the most simple solution. My second suggestion from comment #2 works!

The last version of my add-on uploaded to ATN was 3.2 and it is a webextension for TB68 with a strict_min_version of 68.0a1

Now I wanted to upload a new version for my TB60 users still using the true legacy xul based add-on and this time I just added a dummy manifest.json to the legacy XUL add-on without any other changes with regards to webextension. Just the manifest. It uses the same new version number as the install.rdf (3.4) and the same min/max settings (60.0/60.*).

It was accepted by ATN.

So just by attaching a dummy manifest.json (which is ignored by TB60) I was able to upload a new version for my TB60 users without using the "lower-version-number" hack. Doesn't this solve most of our problems, without having to change anything on ATN or on C-C or running outside of know and tested update paths? Can we make that the official suggestion and put it in the guide?

As a side note: Adding the manifest.json will probably disable the validator for the old legacy addon, as ATN thinks it is a webextension, which are not validated. But this is already happening today with all those "hybrid" add-ons.

Comment 9

[Geoff Lankow (:darktrojan)]

I'll create a dummy legacy API for ESR60 once bug 1552634 is fixed resolved. (Assuming that it will be.) At the moment, that bug changes the possible values for the legacy manifest key, so fixing this bug now will require it to be fixed again later.

Comment 10

[Geoff Lankow (:darktrojan)]

Attachment: 1551831-legacy-dummy-1.diff

This should prevent the code rejecting extensions with a legacy key. The API itself does nothing.

Comment 11

[Magnus Melin [:mkmelin]]

Comment on attachment 9065953 [details] [diff] [review]
1551831-legacy-dummy-1.diff

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

LGTM, r=mkmelin

Comment 12

[Geoff Lankow (:darktrojan)]

Comment on attachment 9065953 [details] [diff] [review]
1551831-legacy-dummy-1.diff

I've changed my mind. This is a silly thing to do.

I misunderstood what was needed here. Adding the dummy API will only serve to silence some warnings. What's actually needed is for the add-on in question to retain its install.rdf.

Comment 13

[Axel Grude [:realRaven]]

I am happy with a WONTFIX / RESOLVED as long as we do not introduce future code that disallows Add-ons that have both manifest and install.rdf - one minor problem is the display of minimum version, but I guess we just need to make the minimum version in manifest.json include 60.0 (in sync with minver supported by install.rdf for all legacy versions that cannot install the manifest file). I believe :Fallen still wants to uplift the legacy changes into a later version of ESR60.