Issue 5316 - Adds dynamic filter types to devtools panel

devtools-panel.js

Index: devtools-panel.js
===================================================================
--- a/devtools-panel.js
+++ b/devtools-panel.js
@@ -14,16 +14,27 @@
  * 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.

+  (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 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.

+    }
+  });
+
 function generateFilter(request, domainSpecific)
 {
   let filter = request.url.replace(/^[\w-]+:\/+(?:www\.)?/, "||");
   let options = [];
 
   if (request.type == "POPUP")
   {
     options.push("popup");