Issue 1601 - Generate blocking filters for all URLs associated with the selected element

include.postload.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 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 180 matching lines @@
 }
 
 // Adds an overlay to an element, which is probably a Flash object
 function addElementOverlay(elt) {
   // If this element is enclosed in an object tag, we prefer to block that instead
   if(!elt)
     return null;
 
   // If element doesn't have at least one of class name, ID or URL, give up
   // because we don't know how to construct a filter rule for it
-  var url = getElementURL(elt);
-  if(!elt.className && !elt.id && !url)
+  if(!hasFilters(elt))
     return;
   var thisStyle = getComputedStyle(elt, null);
   var overlay = document.createElement('div');
   overlay.prisoner = elt;
-  overlay.prisonerURL = url;
   overlay.className = "__adblockplus__overlay";
   overlay.setAttribute('style', 'opacity:0.4; background-color:#ffffff; display:inline-box; ' + 'width:' + thisStyle.width + '; height:' + thisStyle.height + '; position:absolute; overflow:hidden; -webkit-box-sizing:border-box; z-index: 99999');
   var pos = getAbsolutePosition(elt);
   overlay.style.left = pos[0] + "px";
   overlay.style.top = pos[1] + "px";
   // elt.parentNode.appendChild(overlay, elt);
   document.body.appendChild(overlay);
   return overlay;
 }
 
@@ skipping 40 matching lines @@
 // Turn on the choose element to create filter thing
 function clickHide_activate() {
   if(document == null)
     return;
 
   // If we are already selecting, abort now
   if (clickHide_activated || clickHideFiltersDialog)
     clickHide_deactivate();
 
   // Add overlays for elements with URLs so user can easily click them
-  var elts = document.querySelectorAll('object,embed,img,iframe');
+  var elts = document.querySelectorAll('object,embed,img,iframe,video,audio,picture');
   for(var i=0; i<elts.length; i++)
     addElementOverlay(elts[i]);
 
   clickHide_activated = true;
   document.addEventListener("mouseover", clickHide_mouseOver, false);
   document.addEventListener("mouseout", clickHide_mouseOut, false);
   document.addEventListener("click", clickHide_mouseClick, false);
   document.addEventListener("keydown", clickHide_keyDown, false);
 }
 
@@ skipping 47 matching lines @@
   clickHide_mouseClick(ev);
 }
 
 // Hovering over an element so highlight it
 function clickHide_mouseOver(e)
 {
   if (clickHide_activated == false)
     return;
 
   var target = e.target;
-  while (target.parentNode && !(target.id || target.className || target.src || /:.+:/.test(target.getAttribute("style"))))
+  while (target.parentNode && !hasFilters(target))
     target = target.parentNode;
   if (target == document.documentElement || target == document.body)
     target = null;
 
   if (target && target instanceof HTMLElement)
   {
     currentElement = target;
 
     highlightElement(target, "#d6d84b", "#f8fa47");
     target.addEventListener("contextmenu", clickHide_elementClickHandler, false);
@@ skipping 23 matching lines @@
       payload:
       {
         type: "clickhide-deactivate"
       }
     });
     e.preventDefault();
     e.stopPropagation();
   }
 }
 
+
+
 // When the user clicks, the currentElement is the one we want.
 // We should have ABP rules ready for when the
 // popup asks for them.
 function clickHide_mouseClick(e)
 {
   if (!currentElement || !clickHide_activated)
     return;
 
   var elt = currentElement;
-  var url = null;
   if (currentElement.classList.contains("__adblockplus__overlay"))
-  {
     elt = currentElement.prisoner;
-    url = currentElement.prisonerURL;
-  }
-  else if (elt.src)
-    url = elt.src;
 
   clickHideFilters = new Array();
   selectorList = new Array();
 
   var addSelector = function(selector)

[kzar, 2014/11/25 18:10:11]

Why we don't just do "function addSelector(selector)" instead?

[Sebastian Noack, 2014/11/25 19:01:15]

The only reason that comes to my mind would be that the function can not be
called before the two variables above have been defined. Note that normal
function definitions are automatically moved to the top of the scope. Not
necessarily reason enough to leave it that way. However, those lines didn't
change, and I rather don't want to put even more unrelated changes into this
patch.

   {
+    if (selectorList.indexOf(selector) != -1)
+      return;
+
     clickHideFilters.push(document.domain + "##" + selector);
     selectorList.push(selector);
   };
 
   if (elt.id)
     addSelector("#" + escapeCSS(elt.id));
 
   if (elt.classList.length > 0)
   {
     var selector = "";
 
     for (var i = 0; i < elt.classList.length; i++)
       selector += "." + escapeCSS(elt.classList[i]);
 
     addSelector(selector);
   }
 
-  if (url)
+  var urls = getElementURLs(elt);
+  for (var i = 0; i < urls.length; i++)
   {
-    var src = elt.getAttribute("src");
-    var selector = src && escapeCSS(elt.localName) + '[src=' + quote(src) + ']';
+    var url = urls[i];
+    var isHTTP = /^https?:/i.test(url);
 
-    if (/^https?:/i.test(url))
+    if (isHTTP)

[kzar, 2014/11/25 18:10:11]

I found this line a little bit confusing to start with as you're checking if the
URL is HTTP or HTTPS but it looked like you intended to just match HTTP.

[Sebastian Noack, 2014/11/25 19:01:15]

What I actually want, is excluding URLs that can't be handled by request
blocking. So yes, both http:// and https:// are fine, however file://, data:,
javascript: about:blank other special URLs aren't.

I would consider isHTTPorHTTPS as variable name unnecessarily long. And since
HTTPS is just an abbreviation for HTTP+TLS/SSL, it's still HTTP, and therefore I
don't see any wrong implications here.

     {
-      clickHideFilters.push(url.replace(/^[\w\-]+:\/+(?:www\.)?/, "||"));
+      var filter = url.replace(/^[\w\-]+:\/+(?:www\.)?/, "||");
 
-      if (selector)
+      if (clickHideFilters.indexOf(filter) != -1)
+        continue;
+
+      clickHideFilters.push(filter);
+    }
+
+    if (url == elt.src)
+    {
+      var selector = escapeCSS(elt.localName) + '[src=' + quote(elt.getAttribute("src")) + ']';
+
+      if (isHTTP)
         selectorList.push(selector);
+      else
+        addSelector(selector);
     }
-    else if (selector)
-      addSelector(selector);
   }
 
   // restore the original style, before generating the fallback filter that
   // will include the style, and to prevent highlightElements from saving those
   unhighlightElement(currentElement);
 
   // as last resort, create a filter based on inline styles
   if (clickHideFilters.length == 0)
   {
     var style = elt.getAttribute("style");
     if (style)
       addSelector(escapeCSS(elt.localName) + '[style=' + quote(style) + ']');
   }
 
   // Show popup
   clickHide_showDialog(e.clientX, e.clientY, clickHideFilters);
 
   // Highlight the elements specified by selector in yellow
   highlightElements(selectorList.join(","));
   // Now, actually highlight the element the user clicked on in red
   highlightElement(currentElement, "#fd1708", "#f6a1b5");
 
   // Make sure the browser doesn't handle this click
   e.preventDefault();
   e.stopPropagation();
 }
 
-// Extracts source URL from an IMG, OBJECT, EMBED, or IFRAME
-function getElementURL(elt) {
-  // Check children of object nodes for "param" nodes with name="movie" that specify a URL
-  // in value attribute
-  var url;
-  if(elt.localName.toUpperCase() == "OBJECT" && !(url = elt.getAttribute("data"))) {
-    // No data attribute, look in PARAM child tags for a URL for the swf file
-    var params = elt.querySelectorAll("param[name=\"movie\"]");
-    // This OBJECT could contain an EMBED we already nuked, in which case there's no URL
-    if(params[0])
-      url = params[0].getAttribute("value");
-    else {
-      params = elt.querySelectorAll("param[name=\"src\"]");
-      if(params[0])
-        url = params[0].getAttribute("value");
-    }
+function parseSrcSet(element)
+{
+  if (!element.srcset)
+    return [];
 
+  var urls = element.srcset.split(",");
+  for (var i = 0; i < urls.length; i++)
+  {
+    var url = urls[i].replace(/^\s+/, "").replace(/(\s+\S+)?\s*$/, "");
     if (url)
-      url = resolveURL(url);
-  } else if(!url) {
-    url = elt.src || elt.href;
+      urls[i] = resolveURL(url);
+    else
+      urls.splice(i--, 1);

[kzar, 2014/11/25 18:10:11]

Does removing an element from the array you're iterating over cause any problems
in Javascript?

[Sebastian Noack, 2014/11/25 19:01:15]

Not as long as you decrease the iteration counter (that i will have the same
value during next iteration) as done here. Not that this is problamatic in any
programming language, when iterating by index (as opposed to using iterators).

[Sebastian Noack, 2014/11/25 19:03:11]

s/is problamatic/isn't problamatic/

   }
-  return url;
+
+  return urls;
+}
+
+function getElementURLs(elt) {
+  var urls = [];
+
+  if (elt.src)
+    urls.push(elt.src);
+
+  switch (elt.localName)
+  {
+    case "object":
+      var url = elt.getAttribute("data");
+      if (url)
+        return [resolveURL(url)];
+
+      for (var i = 0; i < elt.children.length; i++)
+      {
+        var child = elt.children[i];
+        if (child.localName != "param")
+          continue;
+
+        var name = child.getAttribute("name");
+        if (name != "movie" && name != "src")
+          continue;
+
+        var value = child.getAttribute("value");
+        if (!value)
+          continue;
+
+        return [resolveURL(value)];
+      }
+
+      return [];
+
+    case "video":
+    case "audio":
+    case "picture":
+      for (var i = 0; i < elt.children.length; i++)
+      {
+        var child = elt.children[i];
+
+        if (child.localName != "source")
+          continue;
+
+        if (child.src)
+          urls.push(child.src);
+
+        urls = urls.concat(parseSrcSet(child));
+      }
+
+      break;
+
+    case "img":
+      urls = urls.concat(parseSrcSet(elt));
+  }
+
+  return urls;
+}
+
+function hasFilters(element)

[kzar, 2014/11/26 10:51:22]

Maybe a name like "isFilterable" would be better than "hasFilters"?

[kzar, 2014/11/26 10:51:22]

How come you don't put this function above where it's used? Wouldn't that be
clearer?

[Sebastian Noack, 2014/11/26 11:08:14]

As discussed on IRC, it's called "isBlockable" now.

[Sebastian Noack, 2014/11/26 11:08:14]

Since it relies on "getElementURLs()", I just put it below that function. I
moved both function above now.

+{
+  if (element.id)
+    return true;
+  if (element.classList.length > 0)
+    return true;
+  if (getElementURLs(element).length > 0)
+    return true;
+  if (/:.+:/.test(element.getAttribute("style")))

[kzar, 2014/11/26 10:51:22]

Maybe comment this line to explain what you told me? I don't think it's so
obvious at first glance.

[Sebastian Noack, 2014/11/26 11:08:14]

Well, this logic isn't new. But I guess I can add a comment though.

+    return true;
+
+  return false;
 }
 
 // This function Copyright (c) 2008 Jeni Tennison, from jquery.uri.js
 // and licensed under the MIT license. See jquery-*.min.js for details.
 function removeDotSegments(u) {
   var r = '', m = [];
   if (/\./.test(u)) {
     while (u !== undefined && u !== '') {
       if (u === '.' || u === '..') {
         u = '';
@@ skipping 180 matching lines @@
         break;
       default:
         sendResponse({});
         break;
     }
   });
 
   if (window == window.top)
     ext.backgroundPage.sendMessage({type: "report-html-page"});
 }