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

include.postload.js

Index: include.postload.js
===================================================================
--- a/include.postload.js
+++ b/include.postload.js
@@ -19,45 +19,138 @@
 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,
@@ -183,8 +276,7 @@
   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;
   }
@@ -227,11 +319,8 @@
   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);
   }
 }
@@ -242,9 +331,7 @@
   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);
 }
 
@@ -320,13 +407,11 @@
 
   // 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();
@@ -526,7 +611,6 @@
           // 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);
         }