Issue 2668 - Use new subscription types in options page

options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 192 matching lines @@
           var elements = document.querySelectorAll("[data-access='" + access + "']");
           for (var i = 0; i < elements.length; i++)
           {
             var element = elements[i];
             var control = element.querySelector(".control");
             if (control.localName == "input")
               control.checked = subscription.disabled == false;
             if (subscriptionUrl in recommendationsMap)
             {
               var recommendation = recommendationsMap[subscriptionUrl];
-              if (recommendation.isAdsType)
+              if (recommendation.type == "ads")
               {
                 if (subscription.disabled == false)
                 {
                   collections.allLangs.removeItem(subscription);
                   collections.langs.addItems(subscription);
                 }
                 else
                 {
                   collections.allLangs.addItems(subscription);
                   collections.langs.removeItem(subscription);
@@ skipping 26 matching lines @@
         }
         else
         {
           Object.observe(subscription, onObjectChanged);
         }
 
         var collection = null;
         if (subscriptionUrl in recommendationsMap)
         {
           var recommendation = recommendationsMap[subscriptionUrl];
-          if (recommendation.isPopular)
+          if (recommendation.type != "ads")
             collection = collections.popular;
-          else if (recommendation.isAdsType && subscription.disabled == false)
+          else if (subscription.disabled == false)
             collection = collections.langs;
           else
             collection = collections.allLangs;
         }
         else if (subscriptionUrl == acceptableAdsUrl)
           collection = collections.acceptableAds;
         else
           collection = collections.custom;
 
         collection.addItems(subscription);
@@ skipping 29 matching lines @@
       {
         var element = elements[i];
         var subscription = Object.create(null);
         subscription.title = element.getAttribute("title");
         subscription.url = element.getAttribute("url");
         subscription.disabled = null;
         subscription.downloadStatus = null;
         subscription.homepage = null;
         subscription.lastSuccess = null;
         var recommendation = Object.create(null);
-        recommendation.isAdsType = false;
-        recommendation.isPopular = false;
+        recommendation.type = element.getAttribute("type");
         var prefix = element.getAttribute("prefixes");
         if (prefix)
         {
-          var prefix = element.getAttribute("prefixes").replace(/,/g, "_");
+          prefix = prefix.replace(/\W/g, "_");
           subscription.title = ext.i18n.getMessage("options_language_" + prefix);
-          recommendation.isAdsType = true;
         }
         else
         {
-          subscription.title = element.getAttribute("specialization");
-          recommendation.isPopular = true;
+          var type = recommendation.type.replace(/\W/g, "_");

[Thomas Greiner, 2015/09/25 15:43:22]

I needed to replace "-" with "_" for "common_feature_anti_adblock_title" so to
make it consistent with buildtools I also applied the same method in the block
above: https://hg.adblockplus.org/buildtools/file/tip/packagerChrome.py#l233

[saroyanm, 2015/11/10 16:47:25]

Nit: What is the reason to assign the result to variable if you are not reusing
it ?

[Thomas Greiner, 2015/11/10 19:54:09]

Just a personal preference to improve readability since the variable name
provides context to what this value is supposed to represent. I can replace
those two lines with the following if you prefer that:

subscription.title = ext.i18n.getMessage("common_feature_"
  + recommendation.type.replace(/\W/g, "_") + "_title");

[saroyanm, 2015/11/11 10:47:57]

I see, well while nothing related in coding styles and I can see similar
implementation in the file feel free to keep it as it is right now.

+          subscription.title = ext.i18n.getMessage("common_feature_" + type + "_title");
         }
 
         recommendationsMap[subscription.url] = recommendation;
         updateSubscription(subscription);
       }
     }, false);
     request.send(null);
   }
 
   function onClick(e)
@@ skipping 407 matching lines @@
     filter: ["added", "loaded", "removed"]
   });
   ext.backgroundPage.sendMessage(
   {
     type: "subscriptions.listen",
     filter: ["added", "disabled", "homepage", "removed", "title"]
   });
 
   window.addEventListener("DOMContentLoaded", onDOMLoaded, false);
 })();