Issue 4156 - Adblocking filter only being removed in advanced tab fix

new-options.js

Index: new-options.js
===================================================================
--- a/new-options.js
+++ b/new-options.js
@@ -391,23 +391,6 @@
     }
   ]);
 
-  function updateLanguageCollections(subscription)
-  {
-    if (subscription.recommended == "ads")
-    {
-      if (subscription.disabled)
-      {
-        collections.allLangs.addItems(subscription);
-        collections.langs.removeItem(subscription);
-      }
-      else
-      {
-        collections.allLangs.removeItem(subscription);
-        collections.langs.addItems(subscription);
-      }
-    }
-  }
-
   function addSubscription(subscription)
   {
     var collection;
@@ -432,19 +415,8 @@
 
   function updateSubscription(subscription)
   {
-    var knownSubscription = subscriptionsMap[subscription.url];
-    for (var property in subscription)
-    {
-      if (property == "title" && subscription.recommended)
-        knownSubscription.originalTitle = subscription.title;
-      else
-        knownSubscription[property] = subscription[property];
-    }
-
     for (var name in collections)
-      collections[name].updateItem(knownSubscription);
-
-    return knownSubscription;
+      collections[name].updateItem(subscription);
   }
 
   function updateFilter(filter)
@@ -929,11 +901,35 @@
 
   function onSubscriptionMessage(action, subscription)
   {
+    if (subscription.url in subscriptionsMap)
+    {
+      var knownSubscription = subscriptionsMap[subscription.url];
+      for (var property in subscription)
+      {
+        if (property == "title" && knownSubscription.recommended)
+          knownSubscription.originalTitle = subscription.title;
+        else
+          knownSubscription[property] = subscription[property];
+      }
+      subscription = knownSubscription;
+    }
     switch (action)
     {
       case "disabled":
-        subscription = updateSubscription(subscription);
-        updateLanguageCollections(subscription);
+        if (subscription.recommended == "ads")
+        {
+          if (subscription.disabled == true)
+          {
+            collections.allLangs.addItems(subscription);
+            collections.langs.removeItem(subscription);
+          }
+          else
+          {
+            collections.allLangs.removeItem(subscription);
+            collections.langs.addItems(subscription);
+          }
+        }
+        updateSubscription(subscription);
         break;
       case "downloading":
       case "downloadStatus":
@@ -943,17 +939,19 @@
         updateSubscription(subscription);
         break;
       case "added":
-        if (subscription.url in subscriptionsMap)
-          subscription = updateSubscription(subscription);
+        if (subscription.recommended == "ads")
+        {
+          collections.allLangs.removeItem(subscription);

[Thomas Greiner, 2016/06/22 10:08:04]

The subscription might be disabled when it's added so don't assume that it
isn't. Reverting the merge of `updateLanguageCollections()` should fix that
regression though.

[saroyanm, 2016/06/22 12:31:59]

Well spotted, done

+          collections.langs.addItems(subscription);
+        }
+        else if (subscription.url in subscriptionsMap)
+          updateSubscription(subscription);
         else
           addSubscription(subscription);
-
+        
         collections.filterLists.addItems(subscription);
-        updateLanguageCollections(subscription);
         break;
       case "removed":
-        var knownSubscription = subscriptionsMap[subscription.url];
-
         if (subscription.url == acceptableAdsUrl || subscription.recommended)
         {
           subscription.disabled = true;
@@ -961,10 +959,10 @@
         }
         else
         {
-          collections.custom.removeItem(knownSubscription);
+          collections.custom.removeItem(subscription);
           delete subscriptionsMap[subscription.url];
         }
-        collections.filterLists.removeItem(knownSubscription);
+        collections.filterLists.removeItem(subscription);
         break;
     }