705530 - Support strictCompatibility option in update.rdf

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

To correctly determine which addon updates are compatible when an addon opts-in to strict compatibility checking, the update.rdf needs to specify whether a given update is opting-in. This is because even though we know whether the installed version has opted-in, a newer version may not.

Comment 1

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

This is dependent on the patch in bug 527141 (which in turn is dependent on the patch in bug 693906 - fun, huh?).

The do_timeout() calls in the tests are required because AddonInstall.removeTemperaryFile() isn't called until after onDownloadFailed is sent - so if restartManager() is called too soon, it'll detect a temp file left behind.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1.1

Rebased to correctly apply on top of the updated patch in bug 527141.

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 577558 [details] [diff] [review]
Patch v1.1

(Bah, was sure I set this)

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 577558 [details] [diff] [review]
Patch v1.1

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

::: toolkit/mozapps/extensions/test/xpcshell/test_update_ignorecompat.js
@@ +52,5 @@
>          do_throw("Saw unexpected onNewInstall for " + aInstall.existingAddon.id);
>        do_check_eq(aInstall.version, "4.0");
>      },
>      onDownloadFailed: function(aInstall) {
> +      do_timeout(0, run_test_2);

do_execute_soon please

::: toolkit/mozapps/extensions/test/xpcshell/test_update_strictcompat.js
@@ +1044,5 @@
>          do_throw("Saw unexpected onNewInstall for " + aInstall.existingAddon.id);
>        do_check_eq(aInstall.version, "2.0");
>      },
>      onDownloadFailed: function(aInstall) {
> +      do_timeout(0, run_test_17);

And again

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/integration/fx-team/rev/ac0b65079106

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 577558 [details] [diff] [review]
Patch v1.1

Pe-emptively requesting approval (it's not merged into central yet). Important piece of the compatible-by-default story, required for enabling it on a release version.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

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

Not marking as fixed because this waits for Aurora approval.

Comment 8

[Dave Townsend [:mossop]]

Bugs get marked as fixed when they land in m-c

Comment 9

[Eric Shepherd [:sheppy]]

OK, version is 10 branch, target milestone is 11. Which is it shipping in? :)

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 577558 [details] [diff] [review]
Patch v1.1

[Triage Comment]
Approving low-risk add-ons default to compatible bugs for Aurora with the understanding that we're going to come up with explicit methods of tracking any regressions in aurora/beta related to this and stay on top of them.

Comment 11

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Eric Shepherd [:sheppy] from comment #9)
> OK, version is 10 branch, target milestone is 11. Which is it shipping in? :)

As mentioned on IRC, it's shipping in 10.

Comment 12

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Note: This can't land on Aurora until bug 527141 also has approval and lands, as this is dependent on the patch there.

Comment 13

[Eric Shepherd [:sheppy]]

Docs are updated, fwiw, even though this hasn't quite landed yet.

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Has now!

https://hg.mozilla.org/releases/mozilla-aurora/rev/29d36e0a2564

Comment 15

[u279076]

Is this something QA can verify?

Comment 16

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(Pasting from email)

This applies to add-ons that aren't hosted on AMO, when you have an old version of such an addon installed. If the updated version opts-in to strict compatibility, the addon author should also add the strictCompatibility option to the update.rdf manifest on their server. When the Add-ons Manager checks for updates, it looks at the update.rdf for the latest version of the addon that is compatible - the strictCompatibility option can influence if a version is compatible or not (so the Add-ons Manager knows not to upgrade to a version that wouldn't be compatible).

So to test:
1. Install old version of addon that isn't hosted on AMO
2. That addon's update.rdf should opt-in to strict compatibility, and have a compatibility range that doesn't include the version of Firefox that you're running
3. Check for updates for that addon
4. It should not update to the version that has strict compatibility and doesn't claim compatibility with the version of Firefox that you're running


Unfortunately, I don't know of any addon that uses strictCompatibility yet. I think Henrik Skupin had a litmus test that included a test addon with it's own update.rdf, for testing addon updates - should be able to adapt that.

Comment 17

[Virgil Dicu [:virgil] [QA]]

I'm trying to test this issue while hosting all of the files from Henrik's add-on locally. But I'm a bit stuck before actually adding the StrictCompatibility tag.

I can access the add-ons in Firefox with http://localhost. However, i can't make the update to work in this conditions. I've already added a handler for the xpi file and the install works fine, but when checking for updates nothing happens.

Is there anything else I should modify except for the update links for the add-ons and the update.rdf file?

Comment 18

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I assume because it's not hosted securely over https. Easiest way to get around that for testing is to set extensions.checkUpdateSecurity to false.

Comment 19

[Virgil Dicu [:virgil] [QA]]

Getting the following warning in Error Console when attempting to update at the moment (with update security checking disabled):

Warning: WARN addons.updates: HTTP Request failed for an unknown reason
Source File: resource:///modules/AddonUpdateChecker.jsm
Line: 520

The only modification made to the add-on files was changing the Update Url Links in the two add-ons and in update.rdf so that all lead to the local server, so I'm not that sure in which direction should the problem be. The add-ons can be accessed and installed correctly however. 

I'll play a bit with the about:config options. If this does not work out, will set this to qa- until a normal add-on with the option will be available.