Dave Townsend [:mossop] Assignee
	Description • 14 years ago

Should be pretty straightforward just need a method on AddonManagerPrivate to tell it to update the blocklistState for all add-ons.

	Dave Townsend [:mossop] Assignee
	Comment 1 • 14 years ago

Switches the blocklist service to the new APIs and migrates the tests over for it. It looks like they cover everything I could think of already.

Added a new API to the private add-ons manager, updateAddonAppDisabledStates which makes all providers check and update appDisabled for their add-ons in response to any changes.

	Dave Townsend [:mossop] Assignee
	Comment 2 • 14 years ago

Landed on the project branch: http://hg.mozilla.org/projects/addonsmgr/rev/8221fa8f0870

	Robert Strong (they/them - no direct email)
	Comment 3 • 14 years ago

Comment on attachment 439592 [details] [diff] [review]
patch rev 1

>diff --git a/toolkit/mozapps/extensions/AddonManager.jsm b/toolkit/mozapps/extensions/AddonManager.jsm
>--- a/toolkit/mozapps/extensions/AddonManager.jsm
>+++ b/toolkit/mozapps/extensions/AddonManager.jsm
>@@ -361,16 +361,27 @@ var AddonManagerInternal = {
>    */
>   notifyAddonChanged: function AMI_notifyAddonChanged(aId, aType, aPendingRestart) {
>     this.providers.forEach(function(provider) {
>       callProvider(provider, "addonChanged", null, aId, aType, aPendingRestart);
>     });
>   },
> 
>   /**
>+   * Notifies all providers they need to update the appDisabled property for
>+   * their add-ons in response to some application change such as a blocklist
>+   * update.
nit: s/to some application/to an application/

>+   */
>+  updateAddonAppDisabledStates: function AMI_updateAddonAppDisabledStates() {
>+    this.providers.forEach(function(provider) {
>+      callProvider(provider, "updateAddonAppDisabledStates");
>+    });
>+  },

	Robert Strong (they/them - no direct email)
	Comment 4 • 14 years ago

Comment on attachment 439592 [details] [diff] [review]
patch rev 1

>diff --git a/toolkit/mozapps/extensions/nsBlocklistService.js b/toolkit/mozapps/extensions/nsBlocklistService.js
>--- a/toolkit/mozapps/extensions/nsBlocklistService.js
>+++ b/toolkit/mozapps/extensions/nsBlocklistService.js
>...
>@@ -774,105 +775,124 @@ Blocklist.prototype = {
>     }
> 
>     return Ci.nsIBlocklistService.STATE_NOT_BLOCKED;
>   },
> 
>   _blocklistUpdated: function(oldAddonEntries, oldPluginEntries) {
>     var addonList = [];
> 
>-    var em = Cc["@mozilla.org/extensions/manager;1"].
>-             getService(Ci.nsIExtensionManager);
>-    var addons = em.updateAndGetNewBlocklistedItems();
>+    var self = this;
>+    AddonManager.getAddonsByTypes(["extension", "theme", "locale"], function(addons) {
> 
nit: extra newline

Seems like this should just use getAllAddons and filter out plugins or call getAllAddons and have a single loop handle both. Perhaps a followup?

>-    for (let i = 0; i < addons.length; i++) {
>-      let oldState = -1;
>-      if (oldAddonEntries)
>-        oldState = this._getAddonBlocklistState(addons[i].id, addons[i].version,
>-                                                oldAddonEntries);
>-      let state = this.getAddonBlocklistState(addons[i].id, addons[i].version);
>-      // We don't want to re-warn about items
>-      if (state == oldState)
>-        continue;
>+      for (let i = 0; i < addons.length; i++) {
>+        let oldState = Ci.nsIBlocklistService.STATE_NOTBLOCKED;
>+        if (oldAddonEntries)
>+          oldState = self._getAddonBlocklistState(addons[i].id, addons[i].version,
>+                                                  oldAddonEntries);
>+        let state = self.getAddonBlocklistState(addons[i].id, addons[i].version);
> 
>-      addonList.push({
>-        name: addons[i].name,
>-        version: addons[i].version,
>-        icon: addons[i].iconURL,
>-        disable: false,
>-        blocked: state == Ci.nsIBlocklistService.STATE_BLOCKED,
>-        item: addons[i]
>-      });
>-    }
>+        LOG("Blocklist state for " + addons[i].id + " changed from " + oldState + " to " + state);
nitty nit: the add-on blocklist state won't always change will it?

	Robert Strong (they/them - no direct email)
	Comment 5 • 14 years ago

Comment on attachment 439592 [details] [diff] [review]
patch rev 1

r=me. I only scanned the tests since they were mostly wrapping existing code causing whitespace changes.

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 6 • 14 years ago

I just had the Blocklist warning box pop up, saying that some of my add-ons are known to cause stability or security problems.

But the list was empty.

Looking at the actual list( https://www.mozilla.com/en-US/blocklist/ ), I think my version of the Java Deployment Toolkit actually is in the blocklist, but the warning for bad add-ons was empty.

After Restarting Firefox, it doesn't look like anything was actually disabled.

Is this related to this bug? I'm running the latest trunk build with the new EM code.

	Dave Townsend [:mossop] Assignee
	Comment 7 • 14 years ago

(In reply to comment #6)
> Is this related to this bug? I'm running the latest trunk build with the new EM
> code.

Probably caused by this bug yes, please can you file a new bug report for what you're seeing.

	Dave Townsend [:mossop] Assignee
	Comment 8 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/bb4c7abafcf7

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 9 • 14 years ago

Verified fixed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100517 Minefield/3.7a5pre (.NET CLR 3.5.30729)

I have installed the MetaStream 3 Plugin (npViewpoint.dll) which is on the blocklist. After running Firefox for a while I got the blocklisting dialog and the plugin has been disabled.