[adblockpluschrome] Issue 3596 - Added support for CSS property filters to devtools panel

lib/devtools.js

Index: lib/devtools.js
diff --git a/lib/devtools.js b/lib/devtools.js
index 2fdf5ec8ccde7a433a87bbac8e844519b33fb528..d728f326abd0a9d1fd66551478f1b304fb122b01 100644
--- a/lib/devtools.js
+++ b/lib/devtools.js
@@ -17,7 +17,13 @@
 
 "use strict";
 
-let {RegExpFilter, WhitelistFilter, ElemHideFilter} = require("filterClasses");
+let {

[Sebastian Noack, 2017/01/11 16:20:41]

I think I would prefer to wrap this line like this:

  let {RegExpFilter, WhitelistFilter,
       ElemHideFilter, ElemHideEmulationFilter} = require("filterClasses");

Or like that:

  let {RegExpFilter,
       WhitelistFilter,
       ElemHideFilter,
       ElemHideEmulationFilter} = require("filterClasses");

But I guess this is mostly personal preference, though it is more concise and
looks a little more consistent with the surrounding one-line destructuring
assignments. So let me know if you disagree.

[wspee, 2017/01/12 13:39:58]

Done.

[Sebastian Noack, 2017/02/09 11:07:38]

It seems you haven't changed that code. But since you said "Done", it seems you
don't disagree, but it's rather an accident that this code still looks the same?

[wspee, 2017/02/10 10:46:46]

Done.

+  RegExpFilter,
+  WhitelistFilter,
+  ElemHideFilter,
+  ElemHideEmulationFilter
+} = require("filterClasses");
+
 let {SpecialSubscription} = require("subscriptionClasses");
 let {FilterStorage} = require("filterStorage");
 let {defaultMatcher} = require("matcher");
@@ -160,6 +166,7 @@ exports.logRequest = function(page, url, type, docDomain,
 function logHiddenElements(page, selectors, docDomain)
 {
   let panel = getActivePanel(page);
+  if (panel)
   {
     for (let subscription of FilterStorage.subscriptions)
     {
@@ -168,7 +175,8 @@ function logHiddenElements(page, selectors, docDomain)
 
       for (let filter of subscription.filters)
       {
-        if (!(filter instanceof ElemHideFilter))
+        if (!(filter instanceof ElemHideFilter) &&
+            !(filter instanceof ElemHideEmulationFilter))

[Sebastian Noack, 2017/01/11 16:20:41]

This logic could be slightly simplified (one less logical operation):

  if (!(filter instanceof ElemHideFilter ||
        filter instanceof ElemHideEmulationFilter))

I'm not sure which reads more natural though. I leave it up to you.

[wspee, 2017/01/12 13:39:58]

Done.

           continue;
         if (selectors.indexOf(filter.selector) == -1)
           continue;