Issue 6645 - Collapse elements through user style sheets if possible

include.preload.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 140 matching lines @@
   // setting the style="..." attribute.
   if (element.localName == "video" || element.localName == "audio")
   {
     for (let child of element.children)
     {
       if (child.localName == "source" || child.localName == "track")
         return null;
     }
   }
 
-  let selector = element.localName;
-  let hasSrc = false;

[Manish Jethani, 2018/05/10 01:57:17]

Only one small nit, and this is totally a nit. We could avoid the hasSrc
variable here by setting selector to "" initially, then later:

  if (selector)
    return element.localName + selector;

  return null;

But anyway I don't care about this.

LGTM

[Sebastian Noack, 2018/05/10 11:45:33]

Done.

+  let selector = "";
   for (let attr of ["src", "srcset"])

[Manish Jethani, 2018/05/09 23:11:09]

If I understand correctly [1], srcset is only relevant for <img>. In that case,
the src attribute is treated as if it were part of srcset, and srcset can
override the value of src with a more suitable image.

We have two options:

 1.  Change the order here so that srcset takes precedence. This is problematic
because we may still be blocking the URL specified in src, but we'll end up
hiding based on srcset, and yet if the value of src changes to a non-blocked
URL, the element will remain hidden. Similarly, the element will get displayed
if the value of srcset changes, even though we've blocked the actual image URL
based on src.

 2.  Hide based on both src and srcset (i.e. img[src="foo"][srcset="bar"]. Here
we'll be hiding based on the "aggregate" of src and srcset so that if any of the
values changes, the element gets displayed again. Arguably this is better,
because if the image URL is blocked again after one of these values is updated,
the element will get hidden again.

What do you think?

[1]:
https://developer.mozilla.org/en-US/docs/Learn/HTML/Multimedia_and_embedding/...

[Sebastian Noack, 2018/05/10 01:08:22]

#2 is what my current implementation does. Also consider: If anything changes
that triggers another filter hit and therefore another error event, we just add
another selector to the style sheet with the current implementation.

[Manish Jethani, 2018/05/10 01:57:17]

Ah, I didn't realize you were not breaking from the loop when a selector was
added.

   {
     let value = element.getAttribute(attr);
     if (value && attr in element)
-    {
       selector += "[" + attr + "=" + CSS.escape(value) + "]";
-      hasSrc = true;
-    }
-  }
-
-  return hasSrc ? selector : null;
+  }
+
+  return selector ? element.localName + selector : null;
 }
 
 function hideElement(element)
 {
   function doHide()
   {
     let propertyName = "display";
     let propertyValue = "none";
     if (element.localName == "frame")
     {
@@ skipping 117 matching lines @@
 
             break nodes;
           }
         }
       }
     }
 
     if (selectors.length > 0 || filters.length > 0)
     {
       browser.runtime.sendMessage({
-        type: "devtools.traceElemHide",
+        type: "hitLogger.traceElemHide",
         selectors, filters
       });
     }
   },
 
   onTimeout()
   {
     this.checkNodes(this.changedNodes, this.selectors);
     this.changedNodes = [];
     this.timeout = null;
@@ skipping 226 matching lines @@
   },
 
   hideElements(elements, filters)
   {
     for (let element of elements)
       hideElement(element);
 
     if (this.tracer)
     {
       browser.runtime.sendMessage({
-        type: "devtools.traceElemHide",
+        type: "hitLogger.traceElemHide",
         selectors: [],
         filters
       });
     }
   },
 
   apply()
   {
     browser.runtime.sendMessage({type: "elemhide.getSelectors"}, response =>
     {
@@ skipping 40 matching lines @@
     let element = event.target;
     if (/^i?frame$/.test(element.localName))
       checkCollapse(element);
   }, true);
 }
 
 window.checkCollapse = checkCollapse;
 window.elemhide = elemhide;
 window.typeMap = typeMap;
 window.getURLsFromElement = getURLsFromElement;