Issue 705 - Generate element hiding instead request blocking filters for non-HTTP URLs

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
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 // Click-to-hide stuff
 var clickHide_activated = false;
 var clickHide_filters = null;
 var currentElement = null;
 var clickHideFilters = null;
 var highlightedElementsSelector = null;
 var clickHideFiltersDialog = null;
 var lastRightClickEvent = null;
+
+function quote(value)
+{
+  return '"' + value.replace(/(["\\])/g, "\\$1") + '"';
+}
 
 function supportsShadowRoot(element)
 {
   if (!("createShadowRoot" in element))
     return false;
 
   // There are some elements (e.g. <textarea>), which don't
   // support author created shadow roots and throw an exception.
   var clone = element.cloneNode(false);
   try
@@ skipping 297 matching lines @@
   currentElement.removeEventListener("contextmenu", clickHide_elementClickHandler, false);
 }
 
 // Selects the currently hovered-over filter or cancels selection
 function clickHide_keyDown(e)
 {
   if (!e.ctrlKey && !e.altKey && !e.shiftKey && e.keyCode == 13 /*DOM_VK_RETURN*/)
      clickHide_mouseClick(e);
   else if (!e.ctrlKey && !e.altKey && !e.shiftKey && e.keyCode == 27 /*DOM_VK_ESCAPE*/)
   {
-    clickHide_deactivate();
+    ext.backgroundPage.sendMessage(
+    {
+      type: "forward",
+      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)
 {
@@ skipping 20 matching lines @@
 
   clickHideFilters = new Array();
   selectorList = new Array();
 
   var addSelector = function(selector)
   {
     clickHideFilters.push(document.domain + "##" + selector);
     selectorList.push(selector);
   };
 
-  var addSelectorFromAttribute = function(element, attribute)
-  {
-    var value = element.getAttribute(attribute);
-    if (value)
-      addSelector(element.localName + "[" + attribute + '="' + value.replace(/"/g, '\\"') + '"]');

[Wladimir Palant, 2014/10/24 17:56:55]

This isn't a sufficient escaping approach. First of all, the attribute name
should be escaped as well. Here the corresponding code from Element Hiding
Helper:

function escapeChar(dummy, match)
{
  return "\\" + match.charCodeAt(0).toString(16) + " ";
}

function escapeName(name)
{
  return name.replace(/([^\w\-])/g, "\\$1")
             .replace(/\\([\{\}])/g, escapeChar);
}

The rules for escaping values are even more complicated, e.g. one has to make
sure to escape backslashes. Again, code from Element Hiding Helper:

var escapedValue = attr.selected.replace(/(["\\])/g, '\\$1')
                                .replace(/([\{\}])/g, escapeChar)
                                .replace(/([^\S ])/g, escapeChar);

Frankly, I no longer remember why all of this was necessary. We had lots of
issues with this code and had to adjust it multiple times - we would need to
look at the change history if we want to know the details.

Not that any other selectors the existing code proposes are properly escaped...

[Sebastian Noack, 2014/10/30 16:36:24]

The attribute name is hard-coded in the calling code and needs no escaping. I
restructured the code to make it more obvious.
Backslashes are escaped now. However, I tested it and there is no need to escape
curly brackets and any kind of whitespaces on Chrome and Safari.

-  };
-
   if (elementId)
     addSelector("#" + elementId);
 
   if (elementClasses && elementClasses.length > 0)
   {
     var selector = "";
 
     for (var i = 0; i < elt.classList.length; i++)
       selector += "." + elt.classList[i].replace(/([^\w-])/, "\\$1");
 
     addSelector(selector);
   }
 
   if (url)
   {
+    var src = elt.getAttribute("src");
+    var selector = src && elt.localName + '[src=' + quote(src) + ']';

[Wladimir Palant, 2014/10/30 22:36:28]

elt.localName still needs escaping, DOM methods allow creating elements with any
tag name. And that's an unquoted identifier so the complicated escaping approach
is actually required...

Same thing with the style-based selector below.

[Sebastian Noack, 2014/10/31 13:52:49]

The syntax for valid tag names is very restrictive. It seems that none of the
valid characters needs escaping in CSS. And (at least on Chrome and Safari)
calling document.createElement() with an invalid name raises an exception. Tags
with invalid names given in the HTML source are parsed as  text nodes.

[Wladimir Palant, 2014/11/03 18:14:28]

document.createElement("foo:bar") might still be problematic but not addressing
this here is fine with me.

+
     if (/^https?:/i.test(url))
     {
       clickHideFilters.push(url.replace(/^[\w\-]+:\/+(?:www\.)?/, "||"));
-      selectorList.push('[src="' + elt.getAttribute("src") + '"]');
+
+      if (selector)
+        selectorList.push(selector);
     }
-    else
-      addSelectorFromAttribute(elt, "src");
-  }
-
-  // Show popup
-  clickHide_showDialog(e.clientX, e.clientY, clickHideFilters);
+    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)
-    addSelectorFromAttribute(elt, "style");
+  {
+    var style = elt.getAttribute("style");
+    if (style)
+      addSelector(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();
 }
@@ skipping 173 matching lines @@
           clickHide_activated = true;
           // FIXME: clickHideFilters is erased in clickHide_mouseClick anyway, so why set it?
           clickHideFilters = [msg.filter];
           // Coerce red highlighted overlay on top of element to remove.
           // TODO: Wow, the design of the clickHide stuff is really dumb - gotta fix it sometime
           currentElement = addElementOverlay(target);
           // clickHide_mouseOver(lastRightClickEvent);
           clickHide_mouseClick(lastRightClickEvent);
         }
         else
-          console.log("clickhide-new-filter: URLs don't match. Couldn't find that element.", request.filter, url, lastRightClickEvent.target.src);
+          console.log("clickhide-new-filter: URLs don't match. Couldn't find that element.", msg.filter, url, lastRightClickEvent.target.src);
         break;
       case "clickhide-init":
         if (clickHideFiltersDialog)
         {
           sendResponse({filters: clickHide_filters});
 
           clickHideFiltersDialog.style.width = msg.width + "px";
           clickHideFiltersDialog.style.height = msg.height + "px";
           clickHideFiltersDialog.style.visibility = "visible";
         }
         break;
       case "clickhide-move":
         if (clickHideFiltersDialog)
         {
-          clickHideFiltersDialog.style.left = (parseInt(clickHideFiltersDialog.style.left, 10) + request.x) + "px";
-          clickHideFiltersDialog.style.top = (parseInt(clickHideFiltersDialog.style.top, 10) + request.y) + "px";
+          clickHideFiltersDialog.style.left = (parseInt(clickHideFiltersDialog.style.left, 10) + msg.x) + "px";
+          clickHideFiltersDialog.style.top = (parseInt(clickHideFiltersDialog.style.top, 10) + msg.y) + "px";
         }
         break;
       case "clickhide-close":
         if (clickHideFiltersDialog)
         {
           // Explicitly get rid of currentElement
           if (msg.remove && currentElement && currentElement.parentNode)
             currentElement.parentNode.removeChild(currentElement);
-
-          clickHide_deactivate();
         }
+        clickHide_deactivate();
         break;
       default:
         sendResponse({});
         break;
     }
   });
 
   if (window == window.top)
     ext.backgroundPage.sendMessage({type: "report-html-page"});
 }