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 115 matching lines @@
   {
     if (/^(?!https?:)[\w-]+:/i.test(urls[i]))
       urls.splice(i--, 1);
   }
 
   return urls;
 }
 
 function getSelectorForBlockedElement(element)
 {
+  // Microsoft Edge does not support CSS.escape(). However, it doesn't
+  // support user style sheets either. So the selector would be added
+  // with an author style sheet anyway, which doesn't provide any benefits.
+  if (!("escape" in CSS))
+    return null;
+
   // Setting the "display" CSS property to "none" doesn't have any effect on
   // <frame> elements (in framesets). So we have to hide it inline through
-  // the "visisiblity" CSS property.

[Manish Jethani, 2018/05/07 17:42:02]

Typo: visibility

[Sebastian Noack, 2018/05/09 16:30:46]

Done.

+  // the "visibility" CSS property.
   if (element.localName == "frame")
     return null;
 
   // If the <video> or <audio> element contains any <source> or <track>
   // children, we cannot address it in CSS by the source URL; in that case we
   // don't "collapse" it using a CSS selector but rather hide it directly by
   // 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;
     }
   }
 
-  // Microsoft Edge does not support CSS.escape(). However, it doesn't

[Manish Jethani, 2018/05/07 17:42:02]

Shouldn't this go before the above if block?

[Sebastian Noack, 2018/05/09 16:30:46]

Done.

-  // support user style sheets either. So the selector would be added
-  // with an author style sheet anyway, which doesn't provide any benefits.
-  if (!("escape" in CSS))
-    return null;
-
-  let selector = element.localName;
-  let hasSrc = false;
+  let selector = "";
   for (let attr of ["src", "srcset"])
   {
     let value = element.getAttribute(attr);
     if (value && attr in element)

[Manish Jethani, 2018/05/07 17:42:02]

Any reason why we're checking attr in element here?

[Sebastian Noack, 2018/05/09 16:30:46]

Yes, we only want to consider the src/srcset attribute if its actually supported
by the element, but ignore it if its given in an element where it doesn't have
any effect, e.g. <object src> or <iframe srcset>.

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

Acknowledged.

-    {
       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;