Issue 3870 - Rewrite legacy options page to use async messages

options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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 64 matching lines @@
 var importRawFilters = wrapper({type: "filters.importRaw"},
                                "text", "removeExisting");
 var addFilter = wrapper({type: "filters.add"}, "text");
 var getFilters = wrapper({type: "filters.get"}, "subscriptionUrl");
 var removeFilter = wrapper({type: "filters.remove"}, "text");
 
 var i18n = ext.i18n;
 var whitelistedDomainRegexp = /^@@\|\|([^\/:]+)\^\$document$/;
 var delayedSubscriptionSelection = null;
 
+var acceptableAdsUrl;
+
 // Loads options and sets UI elements accordingly
 function loadOptions()
 {
   // Set page title to i18n version of "Adblock Plus Options"
   document.title = i18n.getMessage("options");
 
   // Set links
   getPref("subscriptions_exceptionsurl", function(url)
   {
-    $("#acceptableAdsLink").attr("href", url);
+    acceptableAdsUrl = url;
+    $("#acceptableAdsLink").attr("href", acceptableAdsUrl);
   });
   getDocLink("acceptable_ads", function(url)
   {
     $("#acceptableAdsDocs").attr("href", url);
   });
   getDocLink("filterdoc", function(url)
   {
     setLinks("filter-must-follow-syntax", url);
   });
   getInfo("application", function(application)
@@ skipping 85 matching lines @@
 $(loadOptions);
 
 // Reloads the displayed subscriptions and filters
 function reloadFilters()
 {
   // Load user filter URLs
   var container = document.getElementById("filterLists");
   while (container.lastChild)
     container.removeChild(container.lastChild);
 
-  getPref("subscriptions_exceptionsurl", function(acceptableAdsUrl)
-  {
-    getSubscriptions(true, false, function(subscriptions)
-    {
-      for (var i = 0; i < subscriptions.length; i++)
-      {
-        var subscription = subscriptions[i];
-        if (subscription.url == acceptableAdsUrl)
-          $("#acceptableAds").prop("checked", !subscription.disabled);
-        else
-          addSubscriptionEntry(subscription);
-      }
-    });
+  getSubscriptions(true, false, function(subscriptions)
+  {
+    for (var i = 0; i < subscriptions.length; i++)
+    {
+      var subscription = subscriptions[i];
+      if (subscription.url == acceptableAdsUrl)
+        $("#acceptableAds").prop("checked", !subscription.disabled);
+      else
+        addSubscriptionEntry(subscription);
+    }
   });
 
   // User-entered filters
   getSubscriptions(false, true, function(subscriptions)
   {
     clearListBox("userFiltersBox");
     clearListBox("excludedDomainsBox");
 
     for (var i = 0; i < subscriptions.length; i++)
       getFilters(subscriptions[i].url, function(filters)
@@ skipping 162 matching lines @@
     addSubscription(url, title, null);
   }
 
   $("#addSubscriptionContainer").hide();
   $("#customSubscriptionContainer").hide();
   $("#addSubscriptionButton").show();
 }
 
 function toggleAcceptableAds()
 {
-  getPref("subscriptions_exceptionsurl", function(url)
-  {
-    toggleSubscription(url, true);
-  });
+  toggleSubscription(acceptableAdsUrl, true);
 }
 
 function findSubscriptionElement(subscription)
 {
   var children = document.getElementById("filterLists").childNodes;
   for (var i = 0; i < children.length; i++)
     if (children[i]._subscription.url == subscription.url)
       return children[i];
   return null;
 }
@@ skipping 56 matching lines @@
     case "disabled":
     case "downloading":
     case "downloadStatus":
     case "homepage":
     case "lastDownload":
     case "title":
       if (element)
         updateSubscriptionInfo(element, subscription);
       break;
     case "added":
-      getPref("subscriptions_exceptionsurl", function(acceptableAdsUrl)
-      {
-        // Hack - Assume newly added subscriptions are currently downloading as
-        //        we process the downloading message before we have finished
-        //        adding the subscription's element!
-        subscription.isDownloading = true;

[Sebastian Noack, 2016/04/07 12:49:14]

That is because you asynchronously retrieve the Acceptable Ads URL. However,
while you wait for a response, the "downloading" notification already arrives.
Note that theoretically, other subscription changes might occur meanwhile as
well, causing similar issues.

How about simply retrieving the Acceptable Ads URL once as very first thing you
do when the options page loads, and caching it in a global variable? That would
not only make this hack redundant, but also reduces the total amount of messages
sent and simplifies the code a bit.

[kzar, 2016/04/07 13:04:53]

Much better solution and works fine. Done.

-
-        if (subscription.url == acceptableAdsUrl)
-          $("#acceptableAds").prop("checked", true);
-        else if (!element)
-          addSubscriptionEntry(subscription);
-      });
+      if (subscription.url == acceptableAdsUrl)
+        $("#acceptableAds").prop("checked", true);
+      else if (!element)
+        addSubscriptionEntry(subscription);
       break;
     case "removed":
-      getPref("subscriptions_exceptionsurl", function(acceptableAdsUrl)
-      {
-        if (subscription.url == acceptableAdsUrl)
-          $("#acceptableAds").prop("checked", false);
-        else if (element)
-          element.parentNode.removeChild(element);
-      });
+      if (subscription.url == acceptableAdsUrl)
+        $("#acceptableAds").prop("checked", false);
+      else if (element)
+        element.parentNode.removeChild(element);
       break;
   }
 }
 
 function onPrefMessage(key, value)
 {
   switch (key)
   {
     case "notifications_showui":
       document.getElementById("shouldShowNotificationsContainer").hidden = !value;
@@ skipping 249 matching lines @@
       onFilterMessage(message.action, message.args[0]);
       break;
     case "prefs.respond":
       onPrefMessage(message.action, message.args[0]);
       break;
     case "subscriptions.respond":
       onSubscriptionMessage(message.action, message.args[0]);
       break;
   }
 });