Issue 1325 - Use Shadow DOM if possible when highlighting page elements

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 currentElement_boxShadow = null;
-var currentElement_backgroundColor;
 var clickHideFilters = null;
 var highlightedElementsSelector = null;
-var highlightedElementsBoxShadows = null;
-var highlightedElementsBGColors = null;
 var clickHideFiltersDialog = null;
 var lastRightClickEvent = null;
 
+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
+  {
+    var root = clone.createShadowRoot();

[Wladimir Palant, 2014/09/22 18:31:09]

Please don't use variables outside the block where they are defined, even if
JavaScript allows it.

[Sebastian Noack, 2014/09/23 11:30:07]

Done, though I don't like that policy. :/

[Wladimir Palant, 2014/09/23 13:29:03]

I think I explained my reasoning before - doing this makes the code less obvious
and more error-prone, developers don't usually assume that a variable declared
inside a block can have side-effects outside it. Besides, it will make switching
to block-scoped variables (something that will hopefully be possible for all our
code at some point) non-trivial.

[Sebastian Noack, 2014/09/23 15:11:39]

Yes, we did. And I think it is pointless to repeat that debate. That is why I
already addressed that comment.
If you come from C or Java this might be unexpected, like a lot of other things
in JavaScript. However, that is how variables work in (almost) all script
languages, including JavaScript, Python, Perl, PHP and Bash. So I think it is
more likely that this is exactly what somebody writing JavaScript assumes.
Just because there will be the "let" statement doesn't mean that we must adopt
it for every single variable declaration in our code. I find function-scoped
variables very useful in some cases like this. It feels hack-ish and stupid, to
add an additional line, to declare a variable in a different scope, just to deny
the concept of function-scoped variables.

[Wladimir Palant, 2014/09/23 15:41:45]

You are wrong about Perl, and in Python and PHP local variables simply aren't
being declared at all (never mind Bash, does it even have local variables?).
Function-scoped variables are one of the least obvious JavaScript features, and
quite a few JavaScript developers don't know about this at all.
Yes, we should - for the reasons outlined above, and to avoid common gotchas
like duplicate variable declarations.

[Sebastian Noack, 2014/09/23 16:37:33]

I don't think so. Run following code if you don't believe:

  sub foo
  {
    if (1)
    {
        my $foo = 42;
    }
    print $foo;
  }
  foo();
You could also say that they're automatically or implicit declared on first
initialization. But the point is that variables in Python and PHP are
function-scoped, too.
Yes, bash has local variables, and they are function-scoped:

  foo ()
  {
    if [ true ]
    then
      local x=42
    fi
    echo $x
  }
  foo
People are often confused with closures and function contexts in JavaScript, but
that is a different topic. But I never heard of somebody who calls himself a
JavaScript coder and has experience in at least one more dynamic language, who
assumed functions to be block-scoped. So I can only guess that this happend to
you at some point, and therefore you consider it a common pitfall. ;)
As outlined above, I don't agree to those reasons.
There is "use strict" and tools like JSLint and JSHint for that purpose.

[Wladimir Palant, 2014/09/23 23:07:39]

Please add "use strict;" at the top.
Still doesn't make it a programming language :)

+  }
+  catch (e)
+  {
+    return false;
+  }
+
+  // There are some elements (e.g. <input>), which support
+  // author created shadow roots, but ignore insertion points.
+  var child = document.createTextNode('');

[Wladimir Palant, 2014/09/22 18:31:09]

Double quotation marks please (same two lines below).

[Sebastian Noack, 2014/09/23 11:30:07]

Done.

+  clone.appendChild(child);
+
+  var shadow = document.createElement('shadow');
+  root.appendChild(shadow);
+
+  return shadow.getDistributedNodes()[0] == child;
+}
+
+function highlightElement(element, shadowColor, backgroundColor)
+{
+  unhighlightElement(element);
+
+  var originalBoxShadowPriority = element.style.getPropertyPriority("box-shadow");
+  var originalBackgroundColorPriority = element.style.getPropertyPriority("background-color");
+  var boxShadow = "inset 0px 0px 5px " + shadowColor;
+
+  var highlightWithStyleAttribute = function()
+  {
+    var originalBoxShadow  = element.style.getPropertyValue("box-shadow");
+    var originalBackgroundColor = element.style.getPropertyValue("background-color");
+
+    element.style.setProperty("box-shadow", boxShadow, "important");
+    element.style.setProperty("background-color", backgroundColor, "important");
+
+    element._unhighlight = function()
+    {
+      this.style.removeProperty("box-shadow");
+      this.style.setProperty(
+        "box-shadow",
+        originalBoxShadow,
+        originalBoxShadowPriority
+      );
+
+      this.style.removeProperty("background-color");
+      this.style.setProperty(
+        "background-color",
+        originalBackgroundColor,
+        originalBackgroundColorPriority
+      );
+    };
+  };
+
+  var highlightWithShadowDOM = function()
+  {
+    var style = document.createElement("style");
+    style.textContent = ":host {" +
+      "box-shadow:" + boxShadow + " !important;" +
+      "background-color:" + backgroundColor + " !important;" +
+    "}";
+
+    var root = element.createShadowRoot();
+    root.appendChild(document.createElement("shadow"));
+    root.appendChild(style);
+
+    element._unhighlight = function()
+    {
+      root.removeChild(style);
+    };
+  };
+
+  if (supportsShadowRoot(element) &&  // Use Shadow DOM if posibble to avoid
+                                      // side effects when the web page updates
+                                      // style while highlighted.

[Wladimir Palant, 2014/09/22 18:31:09]

Please put this comment before the line you are commenting on (and probably
combine it with the comment below) - it's too long to be placed inline.

[Sebastian Noack, 2014/09/23 11:30:07]

Done.

+
+      // However, if the element has important styles we have to replace
+      // the style attribute, and can't override them with shadow DOM.
+      originalBoxShadowPriority       != "important" &&
+      originalBackgroundColorPriority != "important")
+    highlightWithShadowDOM();
+  else
+    highlightWithStyleAttribute();
+}
+
+
+function unhighlightElement(element)
+{
+  if ("_unhighlight" in element)
+  {
+    element._unhighlight();
+    delete element._unhighlight;
+  }
+}
+
 // Highlight elements according to selector string. This would include
 // all elements that would be affected by proposed filters.
 function highlightElements(selectorString) {
-  if(highlightedElementsSelector)
-    unhighlightElements();
+  unhighlightElements();
 
   var highlightedElements = document.querySelectorAll(selectorString);
   highlightedElementsSelector = selectorString;
-  highlightedElementsBoxShadows = new Array();
-  highlightedElementsBGColors = new Array();
 
-  for(var i = 0; i < highlightedElements.length; i++) {
-    highlightedElementsBoxShadows[i] = highlightedElements[i].style.getPropertyValue("-webkit-box-shadow");
-    highlightedElementsBGColors[i] = highlightedElements[i].style.backgroundColor;
-    highlightedElements[i].style.setProperty("-webkit-box-shadow", "inset 0px 0px 5px #fd6738");
-    highlightedElements[i].style.backgroundColor = "#f6e1e5";
-  }
+  for(var i = 0; i < highlightedElements.length; i++)
+    highlightElement(highlightedElements[i], "#fd6738", "#f6e1e5");
 }
 
 // Unhighlight all elements, including those that would be affected by
 // the proposed filters
 function unhighlightElements() {
-  if(highlightedElementsSelector == null)
-    return;
-  var highlightedElements = document.querySelectorAll(highlightedElementsSelector);
-  for(var i = 0; i < highlightedElements.length; i++) {
-    highlightedElements[i].style.setProperty("-webkit-box-shadow", highlightedElementsBoxShadows[i]);
-    highlightedElements[i].style.backgroundColor = highlightedElementsBGColors[i];
+  if (highlightedElementsSelector)
+  {
+    Array.prototype.forEach.call(
+      document.querySelectorAll(highlightedElementsSelector),
+      unhighlightElement
+    );
+
+    highlightedElementsSelector = null;
   }
-  highlightedElementsSelector = null;
 }
 
 // Gets the absolute position of an element by walking up the DOM tree,
 // adding up offsets.
 // I hope there's a better way because it just seems absolutely stupid
 // that the DOM wouldn't have a direct way to get this, given that it
 // has hundreds and hundreds of other methods that do random junk.
 function getAbsolutePosition(elt) {
   var l = 0;
   var t = 0;
@@ skipping 105 matching lines @@
 {
   if (clickHideFiltersDialog)
   {
     document.body.removeChild(clickHideFiltersDialog);
     clickHideFiltersDialog = null;
   }
 
   if(currentElement) {
     currentElement.removeEventListener("contextmenu", clickHide_elementClickHandler, false);
     unhighlightElements();
-    currentElement.style.setProperty("-webkit-box-shadow", currentElement_boxShadow);
-    currentElement.style.backgroundColor = currentElement_backgroundColor;
+    unhighlightElement(currentElement);
     currentElement = null;
     clickHideFilters = null;
   }
   unhighlightElements();
 
   clickHide_activated = false;
   clickHide_filters = null;
   if(!document)
     return; // This can happen inside a nuked iframe...I think
   document.removeEventListener("mouseover", clickHide_mouseOver, false);
@@ skipping 22 matching lines @@
 
   var target = e.target;
   while (target.parentNode && !(target.id || target.className || target.src))
     target = target.parentNode;
   if (target == document.documentElement || target == document.body)
     target = null;
 
   if (target && target instanceof HTMLElement)
   {
     currentElement = target;
-    currentElement_boxShadow = target.style.getPropertyValue("-webkit-box-shadow");
-    currentElement_backgroundColor = target.style.backgroundColor;
-    target.style.setProperty("-webkit-box-shadow", "inset 0px 0px 5px #d6d84b");
-    target.style.backgroundColor = "#f8fa47";
 
+    highlightElement(target, "#d6d84b", "#f8fa47");
     target.addEventListener("contextmenu", clickHide_elementClickHandler, false);
   }
 }
 
 // No longer hovering over this element so unhighlight it
 function clickHide_mouseOut(e)
 {
   if (!clickHide_activated || !currentElement)
     return;
 
-  currentElement.style.setProperty("-webkit-box-shadow", currentElement_boxShadow);
-  currentElement.style.backgroundColor = currentElement_backgroundColor;
-
+  unhighlightElement(currentElement);
   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*/)
   {
@@ skipping 55 matching lines @@
   {
     clickHideFilters.push(relativeToAbsoluteUrl(url));
     selectorList.push(elt.localName + '[src="' + url + '"]');
   }
 
   // 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
-  currentElement.style.setProperty("-webkit-box-shadow", currentElement_boxShadow);
-  currentElement.style.backgroundColor = currentElement_backgroundColor;
+  unhighlightElement(currentElement);
   // Highlight the elements specified by selector in yellow
   highlightElements(selectorList.join(","));
   // Now, actually highlight the element the user clicked on in red
-  currentElement.style.setProperty("-webkit-box-shadow", "inset 0px 0px 5px #fd1708");
-  currentElement.style.backgroundColor = "#f6a1b5";
+  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
@@ skipping 179 matching lines @@
         if(msg.filter === url)
         {
           // This request would have come from the chrome.contextMenu handler, so we
           // simulate the user having chosen the element to get rid of via the usual means.
           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);
-          currentElement_backgroundColor = target.style.backgroundColor;
           // 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);
         break;
       case "clickhide-init":
         if (clickHideFiltersDialog)
         {
           sendResponse({filters: clickHide_filters});
@@ skipping 19 matching lines @@
 
           clickHide_deactivate();
         }
         break;
       default:
         sendResponse({});
         break;
     }
   });
 }