Issue 5316 - Adds dynamic filter types to devtools panel

devtools-panel.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
- * Copyright (C) 2006-2017 eyeo GmbH
+ * 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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 let lastFilterQuery = null;
 

[Jon Sonesen, 2017/08/21 15:31:07]

Should I comment here about the filter-type generation and the reasoning?

[Thomas Greiner, 2017/08/22 11:21:46]

Feel free to if you want.

-ext.backgroundPage.sendMessage({type: "request.getTypes", what: "filterTypes"},

[Thomas Greiner, 2017/08/22 11:21:46]

Detail: You're not retrieving requests but filter types so for consistency with
existing message types I'd suggest using "types.get" or "filterTypes.get" in
which case you will also no longer need to specify the "what" option.

[Jon Sonesen, 2017/08/24 11:29:05]

Acknowledged.

+ext.backgroundPage.sendMessage({type: "types.get"},
   (filterTypes) =>
   {
-    for (let type in filterTypes)

[Thomas Greiner, 2017/08/22 11:21:46]

`filterTypes` is an array but for-in loops are iterating through object
properties. That means that it'd loop through all enumerable properties which
may or may not result in the expected behavior. What you'd want to do here is
use a for-of loop.

[Jon Sonesen, 2017/08/24 11:29:05]

Acknowledged.

-    {
+    let filterTypesElem = document.getElementById("filter-type");
+    let filterStyleElem = document.createElement("style");
+    for (let type of filterTypes)
+    {
+      filterStyleElem.innerHTML +=
+        `#items[data-filter-type=${type}] tr:not([data-type=${type}])` +
+        "{display: none;}";
       let optionNode = document.createElement("option");
-      optionNode.appendChild(document.createTextNode(filterTypes[type]));
-      document.getElementById("filter-type").appendChild(optionNode);

[Thomas Greiner, 2017/08/22 11:21:46]

Suggestion: It's usually good practice to keep DOM operations to a minimum since
those are quite expensive. One way to do that would be by moving
`document.getElementById()` out of the loop and by using
`optionNode.textContent`.

[Jon Sonesen, 2017/08/24 11:29:05]

Acknowledged.

-    }
+      optionNode.appendChild(document.createTextNode(type));
+      filterTypesElem.appendChild(optionNode);
+    }
+    document.body.appendChild(filterStyleElem);
   });
 
 function generateFilter(request, domainSpecific)
 {
   let filter = request.url.replace(/^[\w-]+:\/+(?:www\.)?/, "||");
   let options = [];
 
   if (request.type == "POPUP")
   {
     options.push("popup");
@@ skipping 203 matching lines @@
         break;
     }
   });
 
   // Since Chrome 54 the themeName is accessible, for earlier versions we must
   // assume the default theme is being used.
   // https://bugs.chromium.org/p/chromium/issues/detail?id=608869
   let theme = chrome.devtools.panels.themeName || "default";
   document.body.classList.add(theme);
 }, false);