Issue 700 - Generate filters based on the style attribute as last resort

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 27 matching lines @@
     selectorList.push(selector);
   };
 
   if (elementId)
     addSelector("#" + elementId);
 
   if (elementClasses && elementClasses.length > 0)
   {
     var selector = "";
 
     for (var i = 0; i < elt.classList.length; i++)

[Thomas Greiner, 2014/11/04 09:53:21]

It's certainly not wrong to use elt.classList here but it's inconsistent since
we've been using elementClasses in all of the lines above.

[Sebastian Noack, 2014/11/04 14:51:07]

The patch has already been merged, but I might file a separate issue. However,
I'm in the favor of getting rid of elementClasses (which is initialized by
splitting elt.className with two regexes) and using .classList everywhere
instead. Does anybody know why the original code did it that way in the first
place?

[Thomas Greiner, 2014/11/04 15:15:53]

That's totally fine with me. Looks like the origin of this code can be traced
back to AdThwart when there was no support for classList yet.

[Sebastian Noack, 2014/11/04 15:44:54]

For reference, https://issues.adblockplus.org/ticket/1522

       selector += "." + elt.classList[i].replace(/([^\w-])/, "\\$1");

[Wladimir Palant, 2014/11/03 18:11:33]

Somehow I feel that changing Array.map() into a loop here was a change for
change's sake...

 
     addSelector(selector);
   }
 
   if (url)
   {
     clickHideFilters.push(url.replace(/^[\w\-]+:\/+(?:www\.)?/, "||"));
-    selectorList.push('[src="' + elt.getAttribute("src") + '"]');
-  }
-
-  // Show popup
-  clickHide_showDialog(e.clientX, e.clientY, clickHideFilters);
+
+    var src = elt.getAttribute("src");
+    if (src)
+      selectorList.push('[src=' + quote(src) + ']');
+  }
 
   // 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 && elt.hasAttribute("style"))
-    addSelector(elt.localName + '[style="' + elt.getAttribute("style").replace(/"/g, '\\"') + '"]');
+  if (clickHideFilters.length == 0)
+  {
+    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"});
 }