Last Comment Bug 527141 - Addon update check should take into account compatibility preferences
: Addon update check should take into account compatibility preferences
Status: VERIFIED FIXED
[sg:want P5], [qa!]
: verified-aurora
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: x86 All
: -- major with 27 votes (vote)
: mozilla11
Assigned To: Blair McBride [:Unfocused] (UNAVAILABLE)
:
: Andy McKay [:andym]
Mentors:
https://groups.google.com/forum/#!top...
: 552800 663053 669844 670543 704300 (view as bug list)
Depends on: 698355 707328
Blocks: 692664 707225
  Show dependency treegraph
 
Reported: 2009-11-06 16:05 PST by Jesse Ruderman
Modified: 2012-01-12 15:05 PST (History)
47 users (show)
blair: in‑testsuite+
blair: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
wontfix


Attachments
Patch v1 (44.61 KB, patch)
2011-11-27 04:52 PST, Blair McBride [:Unfocused] (UNAVAILABLE)
no flags Details | Diff | Splinter Review
Patch v1.1 (44.76 KB, patch)
2011-11-28 16:54 PST, Blair McBride [:Unfocused] (UNAVAILABLE)
no flags Details | Diff | Splinter Review
Patch v1.2 (52.05 KB, patch)
2011-11-29 05:16 PST, Blair McBride [:Unfocused] (UNAVAILABLE)
dtownsend: review-
Details | Diff | Splinter Review
Patch v2 (54.00 KB, patch)
2011-12-01 02:55 PST, Blair McBride [:Unfocused] (UNAVAILABLE)
no flags Details | Diff | Splinter Review
Patch v2.0001 (53.80 KB, patch)
2011-12-01 03:11 PST, Blair McBride [:Unfocused] (UNAVAILABLE)
dtownsend: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Test fix (4.25 KB, patch)
2011-12-01 21:41 PST, Blair McBride [:Unfocused] (UNAVAILABLE)
dolske: review+
Details | Diff | Splinter Review

Description Jesse Ruderman 2009-11-06 16:05:06 PST
I have Greasemonkey 0.8.20080609.0.  Firefox hasn't updated me to Greasemonkey 0.8.20090920.2, even though it was released over a month ago.

I suspect this is because I'm using Firefox trunk (3.7a1pre), and the newest version is marked as compatible with 3.5.*.  But the older version of Greasemonkey that I'm using wasn't marked as compatible with 3.7a1pre either!

https://addons.mozilla.org/en-US/firefox/addons/versions/748

It's really lame that beta and nightly users don't get these updates.  It means we never get security fixes for many extensions.  It also means we don't notice incompatibilities between Firefox trunk and newer versions of extensions, which is half the point of beta-testing Firefox.

If there's no version of the extension that's marked as compatible with the version of Firefox I'm using, AMO should assume I want the newest.  (Or the newest among the ones marked as compatible with the newest version of Firefox.)
Comment 2 Dave Townsend [:mossop] 2010-07-03 10:44:38 PDT
*** Bug 552800 has been marked as a duplicate of this bug. ***
Comment 3 Jesse Ruderman 2010-09-19 09:43:06 PDT

*** This bug has been marked as a duplicate of bug 592785 ***
Comment 4 Jesse Ruderman 2010-09-19 09:44:37 PDT
Hmm, or maybe that only fixed the application-update warning, not the extension update.
Comment 5 Henrik Skupin (:whimboo) 2010-09-19 13:57:50 PDT
Is this a regression on trunk?
Comment 6 Wes Kocher (:KWierso) 2010-09-20 17:20:34 PDT
(In reply to comment #5)
> Is this a regression on trunk?

IIRC, it has always been like this. Add-on updates with application maxVersions below the application's actual version number won't be updated, even if compatibility checking is disabled.
Comment 7 Dave Townsend [:mossop] 2010-09-20 17:23:16 PDT
It has always been like this, I think there is a WONTFIX dupe somewhere.
Comment 8 solcroft 2010-09-20 17:26:39 PDT
I really don't see why tradition should matter in this instance, as this is obviously undesirable behavior for reasons mentioned in the first post. This makes sense for stable versions of Firefox, but none for Minefield.
Comment 9 Jesse Ruderman 2010-09-20 17:40:27 PDT
I don't see how the current makes sense even for stable versions of Firefox.  Changing it wouldn't make Firefox any less stable, just more secure.
Comment 10 solcroft 2010-09-20 18:01:21 PDT
(In reply to comment #9)
> I don't see how the current makes sense even for stable versions of Firefox.

Hmm, I'd always been under the impression that the whole point of the current behavior was to prevent updates to extensions that were installed in an earlier version of Fx, but got disabled by compatibility checking after Fx got upgraded to a newer version, and only allow updates after the extensions have been bumped accordingly. Since the extensions are disabled, they affect neither stability nor security.

If it doesn't exist already, perhaps AMO and/or Minefield needs some way for AMO to recognize that it's a test build (as opposed to a stable Fx version) requesting extension updates.
Comment 11 Jesse Ruderman 2010-09-20 18:07:12 PDT
This bug is primarily about extensions that are enabled.  I don't really care what happens to disabled extensions, but it might as well be the same.
Comment 12 Dave Townsend [:mossop] 2010-09-20 18:59:46 PDT
(In reply to comment #8)
> I really don't see why tradition should matter in this instance, as this is
> obviously undesirable behavior for reasons mentioned in the first post. This
> makes sense for stable versions of Firefox, but none for Minefield.

It matters here firstly because we want to identify if we have made an unintentional change with the recent rewrite of the add-ons manager and secondly because if we have made a certain decision in the past (and this was done intentionally) then understanding why we made that decision is important when revisiting it. I didn't go looking for the dupe bug previously because I'm happy to revisit the behaviour, though I do not foresee any change happening in time for Firefox 4.

Unfortunately I can't find anything right now in bug reports that explains for sure why we made the choice we did. I believe that we did it with the idea that when a user was installing an incompatible add-on they would be doing it with the knowledge that it might break things and so they would understand what the problem likely was. They are essentially saying, ok I'll try this version of this add-on out and see if it works.

If it doesn't work presumably they uninstall it. If it works there is no guarantee that a future update that is also incompatible will work so by updating them without decent warning that the update is incompatible then we might be pushing them to a broken version of the add-on.

This is particularly worrisome with the new system doing automatic updates so the user might never even notice their add-on had been updated. Maybe one option would be to make incompatible updates always need to be installed manually and make sure the UI shows that the update is incompatible. This would probably be cheap to do in the new UI.

The rationale breaks down for nightly testers though. For them updating Minefield could quite easily break the incompatible add-on just as much as updating the add-on could so perhaps for nightly builds the restriction could be relaxed entirely. My main concern there would be that nightly testers then aren't testing the same thing that is in release builds.
Comment 13 Tony Mechelynck [:tonymec] 2010-09-20 23:38:06 PDT
I'm testing nightlies (live-testing, not litmus-testing) with a lot of extensions, many of which work with the latest build even though they don't advertise compatibility with it. To use them, I have set the appropriate (and, now, version-dependent) pref in about:config, but I remain ever conscious that it is my responsibility to watch for misbehaving extensions, and if necessary to disable them manually at the slightest doubt. This is particularly true now that big changes have been happening since approx. August 1, breaking an unprecedented number of extensions at a single time. I'm also conscious that extensions labeled as incompatible won't be updated, and that can be expected, since if an extension has really stopped functioning, an update which restores functionality will most probably also have an updated maxVersion (however, a Firefox-only developer might "forget" to update the SeaMonkey maxVersion). To overcome this limitation, I watch the AMO Extensions page (sorted for "recently updated" and with "unreviewed add-ons" displayed), and I read the changelogs of any new versions of extensions I have: if I notice an upgrade to an extension I have disabled (suspecting a malfunction) I may install the upgrade without enabling the suspect extension, if the blurb doesn't fully convince me.

In conclusion, I don't need a change in this behaviour, but I'm conscious that it requires more work on my part (which IMHO is not necessarily a bad thing, as it keeps me conscious of the need to act prudently and responsibly) but I can understand that other testers might feel differently than I do.
Comment 14 Tony Mechelynck [:tonymec] 2010-09-20 23:42:47 PDT
oops: ...since approx. July 1 (not August 1)
Comment 15 DavidC 2010-09-21 02:52:44 PDT
I'm struggling to see any reason why I or any other use would not at least want to be notified that there are updates to my extensions when I have checkCompatibility disabled. The fact that I have disabled this check is effectively a waiver and acceptance of risk that an update might break something. That's my choice.

I've been using Minefield since ~March and needing to manually check each of my ~30 addons to see if they have been updated is not just a nuisance - it prevents me giving timely feedback to the addon developer if they have updated their addon and it causes problems with Minefield.

Strong vote for this to be 'fixed'.
Comment 16 Wayne Mery (:wsmwk, NI for questions) 2010-09-21 04:20:48 PDT
(In reply to comment #12)
> This is particularly worrisome with the new system doing automatic updates so
> the user might never even notice their add-on had been updated. Maybe one
> option would be to make incompatible updates always need to be installed
> manually and make sure the UI shows that the update is incompatible. This would
> probably be cheap to do in the new UI.

I like this idea.
 

I've opted into don't check compatibility, but it does not mean that I don't want notifications/updates for all my add-ons. Quite the contrary. There's enough things to have to pay attention to in this environment to productively use and test the system - receiving update notifications would be a welcome reduction in that workload. Furthermore, at least in this respect, I want my firefox to behave as much as possible as it does for a normal user.
Comment 17 Nicolas Mandil (:NicolasWeb) 2010-09-22 16:03:48 PDT
Bug 556228 is a see also, right ?
Comment 18 Dave Townsend [:mossop] 2011-06-10 09:06:06 PDT
*** Bug 663053 has been marked as a duplicate of this bug. ***
Comment 19 Alex 2011-06-10 09:14:01 PDT
Confirm on x64 windows 7 x64
Comment 20 Dennis McCunney 2011-06-26 19:35:05 PDT
Add me as another vote in favor of a fix.

I currently have 85 extensions installed in Firefox.  I also have Firefox 5.0 release, Aurora, and Nightly installed, all sharing a common profile.  (Only one is active at any one time.)

A good part of my reason for testing beta and nightly builds is precisely to see whether extensions break.  I run Firefox in the first place because the architecture makes it extensible and I *can* install extensions that modify and extend the way it works, and many have become part of my standard kit and crucial to the way I use the browser.

I use Addons Compatibility Reporter to turn off the compatibility checks, and everything works fine.  In practice, I've only ever seen problems when upgrading to a new major release, where underlying changes to Firefox broke things extensions relied on. I have *never* seen an issue with minor or point releases.  When I upgraded to FF 4.0 from FF 3.6, I lost exactly two extensions.  One was an orphan that was no longer developed or supported anyway, and the author of the other promised a fix once FF 4 was released and he wasn't shooting at a moving target.  (He hasn't got one out yet, so it's probably more involved than he suspected.)  No matter, as I found other things to provide the same functionality.

"Extension compatibility" has been a major pain for about as long as FF has existed.  Depending upon what the developer put in maxValue, a simple point release could have Firefox deciding various things were incompatible, when in fact they'd work fine because none of the FF changes touched what the extension did.  I've spent more time elsewhere than I'd care to recall advising people on how to "bump" extensions by editing install.rdf, or installing an extension to turn off the compatibility checks.  Not a few folks got tired of it and migrated to Chrome.

My solution would be simple.  Have Firefox check for compatibility.  If it thinks it's incompatible, give me the option to install it *anyway*.  Put up a message saying "This add-on may be incompatible with this version of Firefox, and may not work as expected.  Install anyway?"  Let *me* decide whether to take the chance.  I shouldn't have to download the XPI file, extract and edit the install.rdf file, stuff it back into the XPI file and install from a local copy, or turn off compatibility checks entirely to keep things working.

Once I *have* said "Install anyway", I should get automatic updates and *not* have to go looking.  If Firefox has it installed, it should look for updates, period.

This is becoming especially critical with Mozilla's new fast release development model, and a new major version every three months.  A whole *lot* of add-ons will break because the developers haven't gotten around to updating the install.rdf file, and even if they have, I believe they still have to get through the AMO review process.  (The developers are generally hobbyists with Real Lives outside of Firefox, so I don't get surprised or upset by tardiness in updates.)

Firefox can't have it both ways.  It can't have AMO trumpeting the literally thousands of add-ons available to enhance the browser, if half of them are likely to stop working every time a new browser version comes out.  The users will get disgusted and go elsewhere, and they'll be right.
______
Dennis
Comment 21 Dave Townsend [:mossop] 2011-07-10 16:53:49 PDT
*** Bug 670543 has been marked as a duplicate of this bug. ***
Comment 22 dindog 2011-07-10 19:23:24 PDT
Rapid Release makes this bug very prominent now, the sooner it being solved, the better
Comment 23 Brian King [:kinger] 2011-08-01 03:52:52 PDT
*** Bug 669844 has been marked as a duplicate of this bug. ***
Comment 24 Allan Gardner (:Mathnerd314) 2011-09-16 09:35:03 PDT
It looks like this isn't fixed server-side, contrary to comment 1: https://versioncheck.addons.mozilla.org/update/VersionCheck.php?reqVersion=2&id={bee6eb20-01e0-ebd1-da83-080329fb9a3a}&version=0.3&maxAppVersion=5.*&status=userEnabled,incompatible&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}&appVersion=9.0a1&appOS=WINNT&appABI=x86-msvc&locale=en-US&currentAppVersion=9.0a1&updateType=97 has no updates (and there is an update, namely 0.3.5, that shows up if you change appVersion to 6.0.1)

The relevant line seems to be https://github.com/jbalogh/zamboni/blob/master/services/update.py#L218, although I don't know if that is what's running on AMO.
Comment 25 rob64rock 2011-09-16 14:06:18 PDT
(In reply to Mathnerd314 from comment #24)
> It looks like this isn't fixed server-side, contrary to comment 1:
> https://versioncheck.addons.mozilla.org/update/VersionCheck.
> php?reqVersion=2&id={bee6eb20-01e0-ebd1-da83-080329fb9a3a}&version=0.
> 3&maxAppVersion=5.*&status=userEnabled,incompatible&appID={ec8030f7-c20a-
> 464f-9b0e-13a3a9e97384}&appVersion=9.0a1&appOS=WINNT&appABI=x86-
> msvc&locale=en-US&currentAppVersion=9.0a1&updateType=97 has no updates (and
> there is an update, namely 0.3.5, that shows up if you change appVersion to
> 6.0.1)
> 
> The relevant line seems to be
> https://github.com/jbalogh/zamboni/blob/master/services/update.py#L218,
> although I don't know if that is what's running on AMO.

If only there was a way to trick the AMO site that you're using Fx 6.0.2 instead of Nightly 9.0a1 ? In regards to doing Add-ons update checks...Changing the Fx UA string has no effect...
Comment 26 Dennis McCunney 2011-09-16 14:17:57 PDT
(In reply to rob64rock from comment #25)
 
> If only there was a way to trick the AMO site that you're using Fx 6.0.2
> instead of Nightly 9.0a1 ? In regards to doing Add-ons update
> checks...Changing the Fx UA string has no effect...

Install the Addons Compatibility Reporter.  I have it here, and when I go to AMO, it warns me the add-on I'm looking to download might not be compatible with the FF version I'm using (which might be FF 7 beta, Aurora, or Nightly), but will let me install it anyway.
Comment 27 rob64rock 2011-09-16 14:57:51 PDT
(In reply to Dennis McCunney from comment #26)
> (In reply to rob64rock from comment #25)
>  
> > If only there was a way to trick the AMO site that you're using Fx 6.0.2
> > instead of Nightly 9.0a1 ? In regards to doing Add-ons update
> > checks...Changing the Fx UA string has no effect...
> 
> Install the Addons Compatibility Reporter.  I have it here, and when I go to
> AMO, it warns me the add-on I'm looking to download might not be compatible
> with the FF version I'm using (which might be FF 7 beta, Aurora, or
> Nightly), but will let me install it anyway.

I already have it, This bug is about when Fx automatically or when you manually checks through "Help > About Firefox" for add-ons updates Fx connects in the background to AMO site servers and they will always return "no updates found" when there may be an newer updated version available, it just happens it's max Fx version compatibility isn't set high enough by the Add-on developer for the Fx AOM to show an "update is available", which means you will have to periodically do manual checks by going to the Add-on's AMO site page and and go to Version Information at the bottom of the page and select 'view all versions' or view the RSS feed page version.

RSS Feed Example: https://addons.mozilla.org/en-US/firefox/addon/googlereaderplus/versions/format:rss
Comment 28 rob64rock 2011-09-16 15:05:19 PDT
(In reply to Dennis McCunney from comment #26)
> (In reply to rob64rock from comment #25)
>  
> > If only there was a way to trick the AMO site that you're using Fx 6.0.2
> > instead of Nightly 9.0a1 ? In regards to doing Add-ons update
> > checks...Changing the Fx UA string has no effect...
> 
> Install the Addons Compatibility Reporter.  I have it here, and when I go to
> AMO, it warns me the add-on I'm looking to download might not be compatible
> with the FF version I'm using (which might be FF 7 beta, Aurora, or
> Nightly), but will let me install it anyway.

Sorry error in my above post there, Bugzilla should provide a way to edit your posts...that's just a thought...

Edited post here:

I already have it, This bug is about when Fx automatically or when you manually check through Fx AOM for add-on updates, Fx connects in the background to AMO site servers and they will always return "no updates found" when there may be an newer updated version available, it just happens it's max Fx version compatibility isn't set high enough by the Add-on developer for the Fx AOM to show an "update is available", which means you will have to periodically do manual checks by going to the Add-on's AMO site page and and go to Version Information at the bottom of the page and select 'view all versions' or view the RSS feed page version.

RSS Feed Example: https://addons.mozilla.org/en-US/firefox/addon/googlereaderplus/versions/format:rss
Comment 29 Thomas Ludwig 2011-09-17 03:16:03 PDT
(In reply to Dennis McCunney from comment #26)
 
> Install the Addons Compatibility Reporter.  I have it here, and when I go to
> AMO, it warns me the add-on I'm looking to download might not be compatible
> with the FF version I'm using (which might be FF 7 beta, Aurora, or
> Nightly), but will let me install it anyway.

As rob64rock already said, it's no problem *installing* add-ons marked as incompatible by using the Addon Compatibility Reporter. The problem is that you won't get any *updates* for them.
Comment 30 rob64rock 2011-09-25 02:33:32 PDT
(In reply to rob64rock from comment #25)
> If only there was a way to trick the AMO site that you're using Fx 6.0.2
> instead of Nightly 9.0a1 ? In regards to doing Add-ons update
> checks...Changing the Fx UA string has no effect...

Good News for those that don't want to wait for Bug 527141 to be RESOLVED....

The extension Add-on Update Checker 1.13b6 added a new about:config pref that is a work around for Bug 527141:
https://addons.mozilla.org/firefox/addon/addon-update-checker/versions/1.13beta6

Chris000001 wrote:

"New version of Add-on Update Checker 1.13b6 adds a hidden (about:config) option to spoof version of Firefox to check for updates (useful for nightly testers.)
extensions.UpdateAddon.SpoofVersion
I recommend setting that string to 6 or 7 (or the SeaMonkey equivalent if you are using SeaMonkey.)
Reset the entry to go back to checking for the actual version of Firefox you are using. This can create problems if you are not using Add-on Compatibility Reporter or setting Firefox so you can install incompatible add-ons, otherwise they will install, but be immediately disabled since they are not compatible."
Comment 31 Willy_ Foo_Foo 2011-09-26 06:08:24 PDT
It's really lame that beta and nightly users don't get these updates.  It means we never get security fixes for many extensions.  It also means we don't notice incompatibilities between Firefox trunk and newer versions of extensions, which is half the point of beta-testing Firefox.

If there's no version of the extension that's marked as compatible with the version of Firefox I'm using, AMO should assume I want the newest.  (Or the newest among the ones marked as compatible with the newest version of Firefox.)


Absolutely agree !!!!!!!!!
Chromium has this(extensions updates regardless of Channel)
& Firefox should enable This !!
I don't see how the current makes sense even for stable versions of Firefox.  Changing it wouldn't make Firefox any less stable, just more secure.
I'm struggling to see any reason why I or any other use would not at least want to be notified that there are updates to my extensions when I have check.Compatibility disabled. The fact that I have disabled this check is effectively a waiver and acceptance of risk that an update might break something. That's my choice.
Have Firefox check for compatibility.  If it thinks it's incompatible, give me the option to install it *anyway*.  Put up a message saying "This add-on may be incompatible with this version of Firefox, and may not work as expected.  Install anyway?"  Let *me* decide whether to take the chance.


Nightly & Aurora Should Give Testers & easy-way to check their addons for problems so they can fix before hand & provide better update
Comment 32 Ziru 2011-09-26 09:19:09 PDT
If a user always sticks to a cutting-edge version of FFx (nightly or aurora), he/she will never get updates for most of the installed addons.

- when the addon v1.0 is compatible with FF6, the user is using FF7 nightly
- when the addon v2.0 is later compatible with FF7, the user has switched to FF8 nightly
- when the addon v3.0 is updated to be compatible with FF8, the user is most likely already on FF9 nightly
....

It is pretty sad for a user to always use a cutting-edge browser with very old addons (v1.0s).
Comment 33 Siddhartha Dugar [:sdrocking] 2011-09-26 09:24:12 PDT
Will the "Addons default to compatible" feature still leave this issue?
Comment 34 Tony Mechelynck [:tonymec] 2011-09-28 06:49:52 PDT
Workaround (not a fix): Periodically check the "Recently updated" pages at AMO, and the home pages of any non-AMO extensions you might be using.
Comment 35 Willy_ Foo_Foo 2011-09-30 23:31:32 PDT
Anyway, Chrome is a fine browser, but as long as they have no review process of their extensions- it is not a serious contender for 1st place

firefox need to make life easier for addons as it is the best thing that make make any browser eat dust
Comment 36 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-10-04 19:29:47 PDT
I just posted on dev-platform with some proposed changes to fix this, asking for objections:
https://groups.google.com/forum/#!topic/mozilla.dev.platform/--eDvY8IRTY
Comment 37 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-11-27 04:52:39 PST
Created attachment 577129 [details] [diff] [review]
Patch v1

This is dependent on the patch in bug 693906.

Contrary to bug 698355 comment 3, I'm purposefully not sending compatMode=strict in the update URL when the addon opts-in to strict compatibility checking. It seems more correct to do that only when strict compatibility checking is enabled globally, since the server doesn't need care about whether the installed version opts in or not. And in fact, this makes it so that an addon can go from opt-in to not opt-in.
Comment 38 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-11-28 16:54:48 PST
Created attachment 577428 [details] [diff] [review]
Patch v1.1

Very minor update to the tests, to fix oranges on Windows/OSX (was calling end_test too early, leaving a temp file behind). No other changes.
Comment 39 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-11-29 01:26:11 PST
*** Bug 704300 has been marked as a duplicate of this bug. ***
Comment 40 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-11-29 05:16:33 PST
Created attachment 577555 [details] [diff] [review]
Patch v1.2

This fixes the issue described in bug 704300.
Comment 41 Dave Townsend [:mossop] 2011-11-29 14:57:41 PST
Comment on attachment 577555 [details] [diff] [review]
Patch v1.2

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

Oh boy, this is going to be fun to merge with the hotfix patch!

Would like to hear answers to these questions before giving this an r+

::: toolkit/mozapps/extensions/AddonUpdateChecker.jsm
@@ +637,5 @@
> + *         The application version to use
> + * @param  aPlatformVersion
> + *         The platform version to use
> + */
> +function findMatchingCompatOverride(aCompatOverrides, aAddonVersion,

Doesn't this already exist in AddonManager.jsm from the other patch?

::: toolkit/mozapps/extensions/XPIProvider.jsm
@@ +1178,5 @@
> +  if (!XPIProvider.checkCompatibility)
> +    compatMode = "ignore";
> +  else if (AddonManager.strictCompatibility)
> +    compatMode = "strict";
> +  uri = uri.replace(/%COMPATIBILITY_MODE%/g, compatMode);

What about aAddon.strictCompatibility?
Comment 42 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-11-29 15:59:49 PST
(In reply to Dave Townsend (:Mossop) from comment #41)
> Comment on attachment 577555 [details] [diff] [review] [diff] [details] [review]
> Patch v1.2
> 
> Review of attachment 577555 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> Oh boy, this is going to be fun to merge with the hotfix patch!

Merging is fun!

> > +function findMatchingCompatOverride(aCompatOverrides, aAddonVersion,
> 
> Doesn't this already exist in AddonManager.jsm from the other patch?

It does... but there's a circular dependency between AddonManasger.jsm and AddonUpdateChecker.jsm :\ It feels wrong to have it only defined in AddonUpdateChecker, and have AddonManager and XPIProvider use that, since it's not really about update checking.

Alternatively, I could create a new AddonsManagerUtils.jsm.

> 
> ::: toolkit/mozapps/extensions/XPIProvider.jsm
> @@ +1178,5 @@
> > +  if (!XPIProvider.checkCompatibility)
> > +    compatMode = "ignore";
> > +  else if (AddonManager.strictCompatibility)
> > +    compatMode = "strict";
> > +  uri = uri.replace(/%COMPATIBILITY_MODE%/g, compatMode);
> 
> What about aAddon.strictCompatibility?

See comment 37.
Comment 43 Dave Townsend [:mossop] 2011-11-29 16:05:12 PST
(In reply to Blair McBride (:Unfocused) from comment #42)
> > > +function findMatchingCompatOverride(aCompatOverrides, aAddonVersion,
> > 
> > Doesn't this already exist in AddonManager.jsm from the other patch?
> 
> It does... but there's a circular dependency between AddonManasger.jsm and
> AddonUpdateChecker.jsm :\ It feels wrong to have it only defined in
> AddonUpdateChecker, and have AddonManager and XPIProvider use that, since
> it's not really about update checking.

Does there have to be a circular dependency? If we lazily imported AddonUpdateChecker.jsm then we wouldn't hit this problem I imagine. It's probably only top-level imported because that was simplest.

> Alternatively, I could create a new AddonsManagerUtils.jsm.

Or I guess maybe stick it into AddonRepository.jsm since that actually parses the compat format.

> > ::: toolkit/mozapps/extensions/XPIProvider.jsm
> > @@ +1178,5 @@
> > > +  if (!XPIProvider.checkCompatibility)
> > > +    compatMode = "ignore";
> > > +  else if (AddonManager.strictCompatibility)
> > > +    compatMode = "strict";
> > > +  uri = uri.replace(/%COMPATIBILITY_MODE%/g, compatMode);
> > 
> > What about aAddon.strictCompatibility?
> 
> See comment 37.

Ok, I get it.
Comment 44 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-01 02:55:37 PST
Created attachment 578216 [details] [diff] [review]
Patch v2
Comment 45 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-01 03:11:34 PST
Created attachment 578221 [details] [diff] [review]
Patch v2.0001

Removed an accidental whitespace change.
Comment 46 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-01 18:35:22 PST
https://hg.mozilla.org/integration/fx-team/rev/b31691b620ba
Comment 47 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-01 18:58:35 PST
Comment on attachment 578221 [details] [diff] [review]
Patch v2.0001

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 48 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-01 21:41:14 PST
Created attachment 578493 [details] [diff] [review]
Test fix

Bah, seems I forgot do_test_pending() in test_update_compatmode.js, so it was passing without doing anything. This patch fixes the test - it still passes, but it does so while actually testing things.
Comment 49 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-01 22:02:10 PST
Test fix: https://hg.mozilla.org/integration/fx-team/rev/b8a8e6953a11
Comment 50 Tim Taubert [:ttaubert] 2011-12-02 05:55:35 PST
https://hg.mozilla.org/mozilla-central/rev/b31691b620ba
Comment 51 Tim Taubert [:ttaubert] 2011-12-02 05:58:03 PST
https://hg.mozilla.org/mozilla-central/rev/b8a8e6953a11
Comment 52 solcroft 2011-12-03 07:34:14 PST
I have extensions.checkCompatibility.nightly set to false, but my outdated extensions are not updating. Am I missing something, and that's not how this patch is supposed to work?

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111203 Firefox/11.0a1
Comment 53 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-07 17:40:30 PST
(In reply to solcroft from comment #52)
> I have extensions.checkCompatibility.nightly set to false, but my outdated
> extensions are not updating. Am I missing something, and that's not how this
> patch is supposed to work?

It's dependent on a change in AMO, bug 698355. That's been fixed, but I found out today that it's not currently enabled on the production site - it should hopefully be enabled this week. Once that change is live, everything should work as intended.
Comment 54 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-07 17:42:06 PST
Note: When I get approval for aurora, I'll roll the patch and the test fix into one changeset and land that.
Comment 55 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-07 19:36:43 PST
Note: This can't land on Aurora until bug 707328 also has approval and can land, as it fixes an issue with the patch here.
Comment 56 Alex Keybl [:akeybl] 2011-12-08 12:25:44 PST
(In reply to Blair McBride (:Unfocused) from comment #55)
> Note: This can't land on Aurora until bug 707328 also has approval and can
> land, as it fixes an issue with the patch here.

This patch is fairly large and possibly not compartmentalized from other add-on code - can you speak to the possibility of regression in this patch? My understanding is that we'd have to back this patch out (as opposed to preffing off the feature) if we find regressions here.
Comment 57 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-08 13:30:15 PST
(In reply to Alex Keybl [:akeybl] from comment #56)
> This patch is fairly large and possibly not compartmentalized from other
> add-on code - can you speak to the possibility of regression in this patch?
> My understanding is that we'd have to back this patch out (as opposed to
> preffing off the feature) if we find regressions here.

Yes, this is the one patch that can't be completely preffed off. It's not a big as it seems though - most of the patch is adding automated tests, and a decent amount is just adding additional  (optional) arguments to functions or changing indentation of existing code. By disabling default-to-compatible, a decent amount of this patch is disabled - about half of what isn't disabled only applies when the user has disabled compatibility checking (which is common on Nightly, but uncommon on Release).

I should note that in addition to being used for default-to-compatible, this patch also fixes an existing security issue for users with compatibility checking disabled, in that previously they would not get security updates for any addons that wouldn't normally be compatible.
Comment 58 Alex Keybl [:akeybl] 2011-12-09 11:20:26 PST
Comment on attachment 578221 [details] [diff] [review]
Patch v2.0001

[Triage Comment]
Per agreement in yesterday's channel meeting, we'll be preffing on add-ons compatible by default early next week. This is a requirement to being feature complete. Approving for Aurora.
Comment 59 Blair McBride [:Unfocused] (UNAVAILABLE) 2011-12-09 19:53:16 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/bc947a914b91
Comment 60 Virgil Dicu [:virgil] [QA] 2011-12-16 05:44:58 PST
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a2) Gecko/20111215 Firefox/10.0a2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0a1) Gecko/20111215 Firefox/11.0a1

Verified on Mac OS 10.6, Windows 7, Windows XP and Ubuntu 11.10 on Firefox 10 Aurora and Firefox 11 Nightly with Pentadactyl (max version of latest version equal to 8.*) for Firefox Aurora and Greasemonkey (max version of latest version equal to 10.*) for Nightly.

STR:
1. Start Firefox 10/11 (Aurora/Nightly)
2. Install an older version of an add-on whose max version is smaller than the application's for both the current version and the latest version to which it updates- e.g. Pentadactyl, version 1.0b61 (https://addons.mozilla.org/en-US/firefox/addon/pentadactyl/versions/)
3. Check for updates in Add-ons Manager.

The add-on from step 2 now updates to the latest version if the default-to-compatible feature is enabled.
Comment 61 nasko_naskov 2011-12-16 08:31:45 PST
How do we enable the default-to-compatible feature?
Comment 62 Lawrence Mandel [:lmandel] (use needinfo) 2011-12-16 08:44:37 PST
default-to-compatible is enabled on Nightly and Aurora. As of next week it will also be enabled on Beta.
Comment 63 Virgil Dicu [:virgil] [QA] 2011-12-20 09:08:35 PST
This also works correctly with compatibility checking disabled (checked with Aurora and Nightly):

1. Disable compatibility checking (add extensions.checkCompatibility.10.0a pref to false in about:config for Aurora and extensions.checkCompatibility.nightly for Nightly)
2. Install an older version of a  binary incompatible add-on and a non-binary add-on (e.g. Pentadactyl and Cooliris (binary))
3. Restart if required.
4. Check for updates for the installed add-ons.

Add-ons update to the newest version with Compatibility checking disabled.

Note You need to log in before you can comment on or make changes to this bug.