Noissue - Load options on DOMContentLoaded rather than jQuery.ready

desktop-options.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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 168 matching lines @@
     filter: ["added", "disabled", "homepage", "lastDownload", "removed",
              "title", "downloadStatus", "downloading"]
   });
 
   // Load recommended subscriptions
   loadRecommendations();
 
   // Show user's filters
   reloadFilters();
 }
-$(document).on("DOMContentLoaded", loadOptions);

[Sebastian Noack, 2017/10/05 19:50:28]

Why not avoiding jQuery at all and just use document.addEventListener? It seems
other code in here does the same.

[Manish Jethani, 2017/10/05 20:10:13]

Yes, that makes sense.

Done.

(I've also moved it towards the bottom of the file, after all the function
definitions.)

 
 function convertSpecialSubscription(subscription)
 {
   for (let filter of subscription.filters)
   {
     if (whitelistedDomainRegexp.test(filter.text))
       appendToListBox("excludedDomainsBox", RegExp.$1);
     else
       appendToListBox("userFiltersBox", filter.text);
   }
@@ skipping 496 matching lines @@
       links[i].setAttribute("target", "_blank");
     }
     else if (typeof args[i] == "function")
     {
       links[i].href = "javascript:void(0);";
       links[i].addEventListener("click", args[i], false);
     }
   }
 }
 
+document.addEventListener("DOMContentLoaded", loadOptions);
+
 ext.onMessage.addListener(message =>

[Manish Jethani, 2017/10/05 20:10:13]

By the way I notice that this listener could get called before the DOM is ready.

[Manish Jethani, 2017/10/05 20:12:34]

It won't in practice though because of the recent changes we made to
showOptions; we can ignore this.

[Sebastian Noack, 2017/10/05 20:14:47]

Unlikely, as the document isn't loaded over the network, but possible. I guess
let's ignore it until we run into any actually reproducible issue, caused by
this, in particular since the options page is eventually replaced. The new
options page might have the same theoretical race condition though.

 {
   switch (message.type)
   {
     case "app.respond":
       switch (message.action)
       {
         case "addSubscription":
           let subscription = message.args[0];
           startSubscriptionSelection(subscription.title, subscription.url);
           break;
@@ skipping 21 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;
   }
 });