401711 - Compatibility check window ignores extensions.update.enabled pref

Status: RESOLVED WONTFIX

(Toolkit :: Add-ons Manager, defect)

Description

[Wladimir Palant]

Steps to reproduce:

1) Make sure extensions.update.enabled preference is false.
2) Upgrade your application.
3) Restart.

Observed results: compatibility check window opens on startup and suggest looking for updated versions.

Expected behavior: no automatic checks so that a XULRunner application can take care of updates itself.

Comment 1

[Wladimir Palant]

Attachment: Proposed patch

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 286684 [details] [diff] [review]
Proposed patch

This won't fix what the bug appears to be about, that will just stop the add-on update notification (which presumably should never happen if the EM isnt looking for updates to addons). 

You want to do the test in or around the call to _showMismatchWindow I believe.

Comment 3

[Wladimir Palant]

Attachment: Patch v1.1

I was indeed looking in the wrong place, sorry. This patch definitely works now, the compatibility check window doesn't show up if extensions.update.enabled is false.

Comment 4

[Alex Vincent [:WeirdAl]]

Sounds like another candidate for a xpcshell regression test :)

Comment 5

[Wladimir Palant]

I don't see how this can be tested (especially with xpcshell).

Comment 6

[Alex Vincent [:WeirdAl]]

http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/extensions/test/unit/head_extensionmanager.js#312

Our EM test harness lets you create a temporary profile, and lets you set preferences on said profile.

Comment 7

[Alex Vincent [:WeirdAl]]

mea culpa, I'm an idiot

Comment 8

[Robert Strong (they/them - no direct email)]

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

I'm not entirely sure we want to do this. On upgrade if a user has extensions that will be disabled due to not being compatible then telling them this should be fine and then they can cancel. This also prevents the synchronization of min / max Version on app upgrade. What is the use case you are trying to solve?

Comment 9

[Robert Strong (they/them - no direct email)]

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

I'm fine with the code but I'm minusing until an answer to comment #8 is provided.

Comment 10

[Robert Strong (they/them - no direct email)]

btw: IMO using this pref to prevent the compatibility sync and notification of incompatible add-ons is not the right way to go. The pref is for automatically checking for updates in the background and is not for preventing the compatibility sync and notification of incompatible add-ons on app upgrade.

Comment 11

[Wladimir Palant]

Use case: XULRunner application that has its own UI for managing add-ons, including an own update check. XULRunner UI that cannot be disabled is very undesirable, especially since our XULRunner builds are not localized. There is also an issue with the functionality - regular update check will respect updateURL setting in the install manifest, something that we don't want to allow for security reasons (AMO doesn't allow it either).

Comment 12

[Robert Strong (they/them - no direct email)]

(In reply to comment #11)
> Use case: XULRunner application that has its own UI for managing add-ons,
> including an own update check. XULRunner UI that cannot be disabled is very
> undesirable, especially since our XULRunner builds are not localized.
I could see this being a new pref then that defaults to performing the check when not present.

> There is
> also an issue with the functionality - regular update check will respect
> updateURL setting in the install manifest, something that we don't want to
> allow for security reasons (AMO doesn't allow it either).
This doesn't parse... AMO doesn't allow AMO installed extensions to provide an updateURL and installing an extension from a non AMO site is no more of a risk than updating the extension now that the update security patch has landed.

Comment 13

[Wladimir Palant]

(In reply to comment #12)
> This doesn't parse... AMO doesn't allow AMO installed extensions to provide an
> updateURL and installing an extension from a non AMO site is no more of a risk
> than updating the extension now that the update security patch has landed.

The reason why AMO disallows updateURL in install manifests has nothing to do with the recent patch. The problem is: somebody could get a harmless extension approved by AMO and then serve malware as an update to it. This update would be downloaded from a third-party site which no longer needs AMO's approval. In our case, ignoring updateURL altogether seems to be the best choice.

Comment 14

[Robert Strong (they/them - no direct email)]

(In reply to comment #13)
> (In reply to comment #12)
> > This doesn't parse... AMO doesn't allow AMO installed extensions to provide an
> > updateURL and installing an extension from a non AMO site is no more of a risk
> > than updating the extension now that the update security patch has landed.
> 
> The reason why AMO disallows updateURL in install manifests has nothing to do
> with the recent patch. The problem is: somebody could get a harmless extension
> approved by AMO and then serve malware as an update to it. This update would be
> downloaded from a third-party site which no longer needs AMO's approval. In our
> case, ignoring updateURL altogether seems to be the best choice.
Please re-read my comment... I understand the problem that AMO addresses by not allowing updateURL to be specified and the review process specifically looking for code that downloads additional code behind the scenes... I never stated that AMO disallowing updateURL had anything to do with the update security patch. I did say that the update security patch has to do with non AMO sites.

It isn't clear to me but it sounds as if you want to be able to prevent updates from non approved sites... is this true? If so, what about installing extensions from non approved sites?

Comment 15

[Wladimir Palant]

> It isn't clear to me but it sounds as if you want to be able
> to prevent updates from non approved sites...

Yes, that's pretty much it.

> If so, what about installing extensions from non approved sites?

This is still open discussion, but we will probably allow updateURL for these extensions only.

Comment 16

[Robert Strong (they/them - no direct email)]

With that as the desired functionality please file an enhancement bug describing what you'd like to accomplish. Resolving -> wontfix per comment #8 and comment #10