Issue 1755 - Create overlays with correct positioning and z-index, considering all ancestors

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
@@ skipping 36 matching lines @@
 {
   return s.replace(/^[\d\-]|[^\w\-\u0080-\uFFFF]/g, escapeChar);
 }
 
 function highlightElement(element, shadowColor, backgroundColor)
 {
   unhighlightElement(element);
 
   var highlightWithOverlay = function()
   {
+    var overlay = addElementOverlay(element);
+
     highlightElement(overlay, shadowColor, backgroundColor);
     overlay.style.pointerEvents = "none";
 
     element._unhighlight = function()
     {
       overlay.parentNode.removeChild(overlay);
     };
   };
 
   var highlightWithStyleAttribute = function()
   {
-    var originalBoxShadow               = element.style.getPropertyValue   ("box-shadow");
-    var originalBoxShadowPriority       = element.style.getPropertyPriority("box-shadow");
-    var originalBackgroundColor         = element.style.getPropertyValue   ("background-color");
+    var originalBoxShadow = element.style.getPropertyValue("box-shadow");
+    var originalBoxShadowPriority = element.style.getPropertyPriority("box-shadow");
+    var originalBackgroundColor = element.style.getPropertyValue("background-color");
     var originalBackgroundColorPriority = element.style.getPropertyPriority("background-color");
 
     element.style.setProperty("box-shadow", "inset 0px 0px 5px " + shadowColor, "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 overlay;
-  if (!("prisoner" in element) && (overlay = addElementOverlay(element)))
+  if ("prisoner" in element)
+    highlightWithStyleAttribute();
+  else
     highlightWithOverlay();
-  else
-    highlightWithStyleAttribute();
 }
 
 
 function unhighlightElement(element)
 {
   if ("_unhighlight" in element)
   {
     element._unhighlight();
     delete element._unhighlight;
   }
@@ skipping 135 matching lines @@
   var t = 0;
   for(; elt; elt = elt.offsetParent) {
     l += elt.offsetLeft;
     t += elt.offsetTop;
   }
   return [l, t];
 }
 
 // Adds an overlay to an element, which is probably a Flash object
 function addElementOverlay(elt) {
-  var zIndex = null;
+  var zIndex = "auto";
   var position = "absolute";
 
   for (var e = elt; e; e = e.parentElement)
   {
     var style = getComputedStyle(e);
 
     // If the element isn't rendered (since its or one of its ancestor's
     // "display" property is "none"), the overlay wouldn't match the element.
     if (style.display == "none")
       return null;
 
     // If the element or one of its ancestors uses fixed postioning, the overlay
     // has to use fixed postioning too. Otherwise it might not match the element.
     if (style.position == "fixed")
       position = "fixed";
 
     // Determine the effective z-index, which is the highest z-index used
     // by the element and its offset ancestors. When using a lower z-index
     // the element would cover the overlay. When using a higher z-index the
     // overlay might also cover other elements.
     if (style.position != "static" && style.zIndex != "auto")
-      zIndex = Math.max(zIndex, parseInt(style.zIndex, 10));

[kzar, 2015/01/13 11:13:13]

zIndex is initially null which when passed to Math.max seems to behave as zero.
Does it matter / is it possible that if the highest zIndex in reality is a
negative number the result will be a zIndex of zero?

[Sebastian Noack, 2015/01/13 13:20:09]

I didn't realize that null is converted to 0, but were assuming that similar to
None in Python, it would be considered smaller than any numeric value. Fixed
that.

+    {
+      if (zIndex == "auto")
+        zIndex = style.zIndex;
+      else
+        zIndex = Math.max(zIndex, style.zIndex);
+    }
   }
 
   var overlay = document.createElement('div');
   overlay.prisoner = elt;
   overlay.className = "__adblockplus__overlay";
   overlay.setAttribute('style', 'opacity:0.4; display:inline-box; overflow:hidden; box-sizing:border-box;');
   var pos = getAbsolutePosition(elt);
   overlay.style.width = elt.offsetWidth + "px";
   overlay.style.height = elt.offsetHeight + "px";
   overlay.style.left = pos[0] + "px";
   overlay.style.top = pos[1] + "px";
   overlay.style.position = position;
-  overlay.style.zIndex = zIndex || "auto";

[kzar, 2015/01/13 11:13:13]

Should possibly check that zIndex is null here instead of just falsey?

[Sebastian Noack, 2015/01/13 13:20:09]

I use "auto" as default value above now.

+  overlay.style.zIndex = zIndex;
 
   // elt.parentNode.appendChild(overlay, elt);
   document.body.appendChild(overlay);
   return overlay;
 }
 
 // Show dialog asking user whether she wants to add the proposed filters derived
 // from selected page element
 function clickHide_showDialog(left, top, filters)
 {
@@ skipping 379 matching lines @@
       case "clickhide-close":
         if (clickHideFiltersDialog && msg.remove)
         {
           // Explicitly get rid of currentElement
           var element = currentElement.prisoner || currentElement;
           if (element && element.parentNode)
             element.parentNode.removeChild(element);
         }
         clickHide_deactivate();
         break;
-      default:

[kzar, 2015/01/13 13:24:50]

Did you remove these lines on purpose?

[Sebastian Noack, 2015/01/13 13:28:20]

I had to rebase. And those lines don't exist anymore upstream.

-        sendResponse({});
-        break;
     }
   });
 
   if (window == window.top)
     ext.backgroundPage.sendMessage({type: "report-html-page"});
 }