Issue 5316 - Adds dynamic filter types to devtools panel

messageResponder.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 25 matching lines @@
   const {Synchronizer} = require("synchronizer");
 
   const info = require("info");
   const {
     Subscription,
     DownloadableSubscription,
     SpecialSubscription
   } = require("subscriptionClasses");
 
   port.on("types.get", (message, sender) =>
-    Array.from(require("requestBlocker").filterTypes));

[Sebastian Noack, 2017/09/05 18:12:14]

I wonder whether it would be worth to make sure that "OTHER" is the last item in
the list, and if so whether this should be done here or in adblockpluschrome. If
we don't explicitly handle this, "OTHER" will appear somewhere in the middle of
the list, which might appear weird/confusing in the user interface.

[Thomas Greiner, 2017/09/06 11:00:25]

Good point. We're currently not imposing any order on the items so I think we
can leave it the way it is though.

If we want to start ordering items, I agree with the change you suggested. Such
a change would be something I'd consider to be part of the UI code since it's
only relevant for how information is laid out in the UI.

[Jon Sonesen, 2017/09/06 13:12:15]

What about removing the OTHER return from lib/requestBlocker and then hard
coding it into devtool-panel.html right after any, so you have any and other
appearing next to each other at the top of the list?

[Sebastian Noack, 2017/09/06 17:39:19]

I don't think the order matters much, except for OTHER, which is just weird to
have in the middle of the list. So why not just move it to the end of the list?
Well, before this change, the items were in a hard-coded order, with OTHER being
the last item. So if we agree that OTHER being last was not just coincidental,
changing this would be a UX regression.

On 2017/09/06 13:12:15, Jon Sonesen wrote:
Then requestBlocker.filterTypes would be incomplete/inconsistent which might be
confusing and likely results in bugs if we should ever use it somewhere else.
Also it doesn't seem any less code overall than just changing the order.

[Jon Sonesen, 2017/09/07 07:46:17]

Yeah, I agree here. Will do.

[Thomas Greiner, 2017/09/12 13:06:16]

Good point. In that case no objections from my end.

+  {
+    let filterTypes = Array.from(require("requestBlocker").filterTypes);
+    filterTypes.push(...filterTypes.splice(filterTypes.indexOf("OTHER"), 1));
+    return filterTypes;
+  });
 
   // Some modules doesn't exist on Firefox. Moreover,
   // require() throws an exception on Firefox in that case.
   // However, try/catch causes the whole function to to be
   // deoptimized on V8. So we wrap it into another function.
   function tryRequire(module)
   {
     try
     {
       return require(module);
@@ skipping 380 matching lines @@
     if (message.url)
       subscriptions = [Subscription.fromURL(message.url)];
 
     for (let subscription of subscriptions)
     {
       if (subscription instanceof DownloadableSubscription)
         Synchronizer.execute(subscription, true);
     }
   });
 })(this);