347140 - blocklisting broken sometime after implementation

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

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

hasAttribute is failing in the blocklist code... the blocklist code hasn't changed so something else must have. I've got it working in my tree now and will submit a patch within a day.

Comment 1

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

Attachment: Initial badness found

So, we have been adding the pref observer for quite some time before profile-after-change so when we load the profile the user prefs are loaded which then notifies the pref observer before classinfo has loaded. If the pref to check for compatibility is false we then notify the observer and then verify whether the installed extensions should be disabled / enabled for the app. This tries to load the blocklist... hence the error.

Comment 2

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

Attachment: patch

Comment 3

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

Attachment: same patch -b

Comment 4

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

Comment on attachment 232096 [details] [diff] [review]
patch

>Index: mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
...
>-        var blocklistElement = itemNodes.item(i);
>+        var blocklistElement = itemNodes[i];
I switched to the shorter syntax here and elsewhere.

>@@ -2521,18 +2521,18 @@
...
> 
>   _getItemNodes: function(deChildNodes) {
>     const kELEMENT_NODE = Components.interfaces.nsIDOMNode.ELEMENT_NODE;
>     for (var i = 0; i < deChildNodes.length; ++i) {
>-      var emItemsElement = deChildNodes.item(i);
>-      if (emItemsElement.nodeType == kELEMENT_NODE ||
>+      var emItemsElement = deChildNodes[i];
>+      if (emItemsElement.nodeType == kELEMENT_NODE &&
>           emItemsElement.localName == "emItems")
This wasn't caught earlier since localName should always equal emItems

>@@ -2544,59 +2544,63 @@
...
>+  var found = false;
> 
>+  if (versionRangeElement) {
>+    for (var i = 0; i < versionRangeElement.childNodes.length; ++i) {
>+      var targetAppElement = versionRangeElement.childNodes[i];
>+      if (targetAppElement.nodeType != Components.interfaces.nsIDOMNode.ELEMENT_NODE ||
>+          targetAppElement.localName != "targetApplication")
>+        continue;
>+      found = true;
>+      // default to the current application if id is not provided.
>+      var appID = targetAppElement.hasAttribute("id") ? targetAppElement.getAttribute("id") : gApp.ID;
>+      this.targetApps[appID] = this.getBlocklistAppVersions(targetAppElement);
>+    }
>+  }
>   // Default to all versions of the extension and the current application when
>   // versionRange is not defined.
>-  if (!versionRangeElement || versionRangeElement.childNodes.length == 0) {
>+  if (!found)
Protect against the xml having text nodes here and in the next chunk.

>@@ -2634,19 +2638,16 @@
>   gPref = Components.classes["@mozilla.org/preferences-service;1"]
>                     .getService(Components.interfaces.nsIPrefBranch2);
>-  gLoggingEnabled = getPref("getBoolPref", PREF_EM_LOGGING_ENABLED, false);
>-  gCheckCompatibility = getPref("getBoolPref", PREF_EM_CHECK_COMPATIBILITY, true);
>-  gPref.addObserver("extensions.", this, false);
We were adding our pref observer before profile-after-change which meant that we would get the default values for these prefs, reset them after profile-after-change, and also receive notifications for all prefs under 'extensions.' that are different than the app's default. These are moved by this patch to _profileSelected in the remainder of this patch.

Comment 5

[Mike Schroepfer]

Robert - please land on trunk.

Comment 6

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

oops... this already landed on trunk and I forgot to mark this fixed.

Comment 7

[Mike Schroepfer]

Comment on attachment 232096 [details] [diff] [review]
patch

a=schrep for drivers.

Comment 8

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

Checked in to MOZILLA_1_8_BRANCH