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

include.postload.js

Index: include.postload.js
===================================================================
--- a/include.postload.js
+++ b/include.postload.js
@@ -24,6 +24,11 @@
 var clickHideFiltersDialog = null;
 var lastRightClickEvent = null;
 
+function quote(value)
+{
+  return '"' + value.replace(/(["\\])/g, "\\$1") + '"';
+}
+
 function supportsShadowRoot(element)
 {
   if (!("createShadowRoot" in element))
@@ -309,7 +314,7 @@
     return;
 
   var target = e.target;
-  while (target.parentNode && !(target.id || target.className || target.src))
+  while (target.parentNode && !(target.id || target.className || target.src || /:.+:/.test(target.getAttribute("style"))))
     target = target.parentNode;
   if (target == document.documentElement || target == document.body)
     target = null;
@@ -381,33 +386,50 @@
 
   clickHideFilters = new Array();
   selectorList = new Array();
+
+  var addSelector = function(selector)
+  {
+    clickHideFilters.push(document.domain + "##" + selector);
+    selectorList.push(selector);
+  };
+
   if (elementId)
-  {
-    clickHideFilters.push(document.domain + "###" + elementId);
-    selectorList.push("#" + elementId);
-  }
+    addSelector("#" + elementId);
+
   if (elementClasses && elementClasses.length > 0)
   {
-    var selector = elementClasses.map(function(elClass)
-    {
-      return "." + elClass.replace(/([^\w-])/, "\\$1");
-    }).join("");
+    var selector = "";
 
-    clickHideFilters.push(document.domain + "##" + selector);
-    selectorList.push(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") + '"]');
+
+    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)
+  {
+    var style = elt.getAttribute("style");
+    if (style)
+      addSelector(elt.localName + '[style=' + quote(style) + ']');
   }
 
   // Show popup
   clickHide_showDialog(e.clientX, e.clientY, clickHideFilters);
 
-  // Highlight the unlucky elements
-  // Restore currentElement's box-shadow and bgcolor so that highlightElements won't save those
-  unhighlightElement(currentElement);
   // Highlight the elements specified by selector in yellow
   highlightElements(selectorList.join(","));
   // Now, actually highlight the element the user clicked on in red