Issue 1610 - Don't remove overlays before the "Block element" dialog is closed

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 157 matching lines @@
   {
     Array.prototype.forEach.call(
       document.querySelectorAll(highlightedElementsSelector),
       unhighlightElement
     );
 
     highlightedElementsSelector = null;
   }
 }
 
+function getElementURLs(element) {
+  var urls = [];
+
+  if (element.src)
+    urls.push(element.src);
+
+  switch (element.localName)
+  {
+    case "object":
+      var url = element.getAttribute("data");
+      if (url)
+        return [resolveURL(url)];
+
+      for (var i = 0; i < element.children.length; i++)
+      {
+        var child = element.children[i];
+        if (child.localName != "param")
+          continue;
+
+        var name = child.getAttribute("name");
+        if (name != "movie" && name != "src")
+          continue;
+
+        var value = child.getAttribute("value");
+        if (!value)
+          continue;
+
+        return [resolveURL(value)];
+      }
+
+      return [];
+
+    case "video":
+    case "audio":
+    case "picture":
+      for (var i = 0; i < element.children.length; i++)
+      {
+        var child = element.children[i];
+
+        if (child.localName != "source")
+          continue;
+
+        if (child.src)
+          urls.push(child.src);
+
+        urls = urls.concat(parseSrcSet(child));
+      }
+
+      if (element.poster)
+        urls.push(element.poster);
+
+      break;
+
+    case "img":
+      urls = urls.concat(parseSrcSet(element));
+  }
+
+  return urls;
+}
+
+function isBlockable(element)
+{
+  if (element.id)
+    return true;
+  if (element.classList.length > 0)
+    return true;
+  if (getElementURLs(element).length > 0)
+    return true;
+
+  // We only generate filters based on the "style" attribute,
+  // if this is the only way we can generate a filter, and
+  // only if there are at least two CSS properties defined.
+  if (/:.+:/.test(element.getAttribute("style")))
+    return true;
+
+  return false;
+}
+
 // 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;
   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) {
   // If this element is enclosed in an object tag, we prefer to block that instead
   if(!elt)
     return null;
 
   // If element doesn't have at least one of class name, ID or URL, give up
   // because we don't know how to construct a filter rule for it
-  if(!hasFilters(elt))
+  if(!isBlockable(elt))
     return;
+
+  // If the element isn't rendered (since its or one of its ancestor's
+  // "diplay" property is "none"), the overlay wouldn't match the element.
+  if (!elt.offsetParent)
+    return;
+
   var thisStyle = getComputedStyle(elt, null);
   var overlay = document.createElement('div');
   overlay.prisoner = elt;
   overlay.className = "__adblockplus__overlay";
-  overlay.setAttribute('style', 'opacity:0.4; background-color:#ffffff; display:inline-box; ' + 'width:' + thisStyle.width + '; height:' + thisStyle.height + '; position:absolute; overflow:hidden; -webkit-box-sizing:border-box; z-index: 99999');
+  overlay.setAttribute('style', 'opacity:0.4; background-color:#ffffff; display:inline-box; ' + 'width:' + thisStyle.width + '; height:' + thisStyle.height + '; position:absolute; overflow:hidden; -webkit-box-sizing:border-box;');
   var pos = getAbsolutePosition(elt);
   overlay.style.left = pos[0] + "px";
   overlay.style.top = pos[1] + "px";
+
+  if (thisStyle.position != "static")
+    overlay.style.zIndex = thisStyle.zIndex;
+  else
+    overlay.style.zIndex = getComputedStyle(elt.offsetParent).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)
 {
   // If we are already selecting, abort now
@@ skipping 53 matching lines @@
 // on whether the user actually wants these filters
 function clickHide_rulesPending() {
   clickHide_activated = false;
   document.removeEventListener("mouseover", clickHide_mouseOver, true);
   document.removeEventListener("mouseout", clickHide_mouseOut, true);
   document.removeEventListener("click", clickHide_mouseClick, true);
   document.removeEventListener("keydown", clickHide_keyDown, true);
 }
 
 // Turn off click-to-hide
 function clickHide_deactivate(keepOverlays)

[kzar, 2014/11/26 14:14:04]

We're relying on the legacy code not passing this parameter and it therefore
being undefined?

[Sebastian Noack, 2014/11/26 14:41:39]

Correct. I tried to keep the patch as compact as possible this time.

 {
   if (clickHideFiltersDialog)
   {
     document.body.removeChild(clickHideFiltersDialog);
     clickHideFiltersDialog = null;
   }
 
   clickHide_activated = false;
   clickHide_filters = null;
   if(!document)
     return; // This can happen inside a nuked iframe...I think
   document.removeEventListener("mouseover", clickHide_mouseOver, true);
   document.removeEventListener("mouseout", clickHide_mouseOut, true);
   document.removeEventListener("click", clickHide_mouseClick, true);
   document.removeEventListener("keydown", clickHide_keyDown, true);
 
   if (!keepOverlays)
   {
-    if(currentElement) {

[kzar, 2014/11/26 14:14:04]

Nitpick: Should be space between "if" and "("?

[Sebastian Noack, 2014/11/26 14:41:39]

I just copied the code from above, but I have added the space now.

-      currentElement.removeEventListener("contextmenu", clickHide_elementClickHandler, false);

[kzar, 2014/11/26 14:14:04]

Last parameter should be true?

[Sebastian Noack, 2014/11/26 14:41:39]

Yes, this patch was created on top of a version, were it still be false. Should
be fixed after rebase.

+    if (currentElement) {
+      currentElement.removeEventListener("contextmenu",  clickHide_elementClickHandler, true);
       unhighlightElements();
       unhighlightElement(currentElement);
       currentElement = null;
       clickHideFilters = null;
     }
     unhighlightElements();
 
     var overlays = document.getElementsByClassName("__adblockplus__overlay");
     while (overlays.length > 0)
       overlays[0].parentNode.removeChild(overlays[0]);
   }
 }
 
 function clickHide_elementClickHandler(ev) {
   ev.preventDefault();
   ev.stopPropagation();
   clickHide_mouseClick(ev);
 }
 
 // Hovering over an element so highlight it
 function clickHide_mouseOver(e)
 {
   if (clickHide_activated == false)
     return;
 
   var target = e.target;
-  while (target.parentNode && !hasFilters(target))
+  while (target.parentNode && !isBlockable(target))
     target = target.parentNode;
   if (target == document.documentElement || target == document.body)
     target = null;
 
   if (target && target instanceof HTMLElement)
   {
     currentElement = target;
 
     highlightElement(target, "#d6d84b", "#f8fa47");
     target.addEventListener("contextmenu", clickHide_elementClickHandler, true);
@@ skipping 23 matching lines @@
       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)
 {
   if (!currentElement || !clickHide_activated)
     return;
 
   var elt = currentElement;
   if (currentElement.classList.contains("__adblockplus__overlay"))
@@ skipping 21 matching lines @@
     for (var i = 0; i < elt.classList.length; i++)
       selector += "." + escapeCSS(elt.classList[i]);
 
     addSelector(selector);
   }
 
   var urls = getElementURLs(elt);
   for (var i = 0; i < urls.length; i++)
   {
     var url = urls[i];
-    var isHTTP = /^https?:/i.test(url);
-
-    if (isHTTP)
+
+    if (/^https?:/i.test(url))
     {
       var filter = url.replace(/^[\w\-]+:\/+(?:www\.)?/, "||");
 
-      if (clickHideFilters.indexOf(filter) != -1)
-        continue;
-
-      clickHideFilters.push(filter);
+      if (clickHideFilters.indexOf(filter) == -1)
+        clickHideFilters.push(filter);
+
+      continue;
     }
 
     if (url == elt.src)
-    {
-      var selector = escapeCSS(elt.localName) + '[src=' + quote(elt.getAttribute("src")) + ']';
-
-      if (isHTTP)
-        selectorList.push(selector);
-      else
-        addSelector(selector);
-    }
+      addSelector(escapeCSS(elt.localName) + '[src=' + quote(elt.getAttribute("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");
@@ skipping 24 matching lines @@
   for (var i = 0; i < urls.length; i++)
   {
     var url = urls[i].replace(/^\s+/, "").replace(/(\s+\S+)?\s*$/, "");
     if (url)
       urls[i] = resolveURL(url);
     else
       urls.splice(i--, 1);
   }
 
   return urls;
-}
-
-function getElementURLs(elt) {
-  var urls = [];
-
-  if (elt.src)
-    urls.push(elt.src);
-
-  switch (elt.localName)
-  {
-    case "object":
-      var url = elt.getAttribute("data");
-      if (url)
-        return [resolveURL(url)];
-
-      for (var i = 0; i < elt.children.length; i++)
-      {
-        var child = elt.children[i];
-        if (child.localName != "param")
-          continue;
-
-        var name = child.getAttribute("name");
-        if (name != "movie" && name != "src")
-          continue;
-
-        var value = child.getAttribute("value");
-        if (!value)
-          continue;
-
-        return [resolveURL(value)];
-      }
-
-      return [];
-
-    case "video":
-    case "audio":
-    case "picture":
-      for (var i = 0; i < elt.children.length; i++)
-      {
-        var child = elt.children[i];
-
-        if (child.localName != "source")
-          continue;
-
-        if (child.src)
-          urls.push(child.src);
-
-        urls = urls.concat(parseSrcSet(child));
-      }
-
-      if (elt.poster)
-        urls.push(elt.poster);
-
-      break;
-
-    case "img":
-      urls = urls.concat(parseSrcSet(elt));
-  }
-
-  return urls;
-}
-
-function hasFilters(element)
-{
-  if (element.id)
-    return true;
-  if (element.classList.length > 0)
-    return true;
-  if (getElementURLs(element).length > 0)
-    return true;
-  if (/:.+:/.test(element.getAttribute("style")))
-    return true;
-
-  return false;
 }
 
 // This function Copyright (c) 2008 Jeni Tennison, from jquery.uri.js
 // and licensed under the MIT license. See jquery-*.min.js for details.
 function removeDotSegments(u) {
   var r = '', m = [];
   if (/\./.test(u)) {
     while (u !== undefined && u !== '') {
       if (u === '.' || u === '..') {
         u = '';
@@ skipping 142 matching lines @@
         break;
       default:
         sendResponse({});
         break;
     }
   });
 
   if (window == window.top)
     ext.backgroundPage.sendMessage({type: "report-html-page"});
 }