Issue #350 - iFrame block element improvements

include.postload.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 highlightedElementsSelector = null;
 var clickHideFiltersDialog = null;
 var lastRightClickEvent = null;
+var lastRightClickEventRandom = null;
 
 function escapeChar(chr)
 {
   var code = chr.charCodeAt(0);
 
   // Control characters and leading digits must be escaped based on
   // their char code in CSS. Moreover, curly brackets aren't allowed
   // in elemhide filters, and therefore must be escaped based on their
   // char code as well.
   if (code <= 0x1F || code == 0x7F || /[\d\{\}]/.test(chr))
@@ skipping 347 matching lines @@
   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)
   {
+    lastRightClickEvent = null;
+
     if (currentElement) {
       currentElement.removeEventListener("contextmenu",  clickHide_elementClickHandler, true);
       unhighlightElements();
       unhighlightElement(currentElement);
       currentElement = null;
     }
     unhighlightElements();
 
     var overlays = document.getElementsByClassName("__adblockplus__overlay");
     while (overlays.length > 0)
@@ skipping 115 matching lines @@
   }
 
   // as last resort, create a filter based on inline styles
   if (clickHideFilters.length == 0)
   {
     var style = elt.getAttribute("style");
     if (style)
       addSelector(escapeCSS(elt.localName) + '[style=' + quote(style) + ']');
   }
 
-  // Show popup
-  clickHide_showDialog(e.clientX, e.clientY, clickHideFilters);
+  // Show popup, or if inside frame tell the parent to do it
+  if (window.self == window.top)
+    clickHide_showDialog(e.clientX, e.clientY, clickHideFilters);
+  else
+    ext.backgroundPage.sendMessage(
+    {
+      type: "clickHide-showDialog",
+      screenX: e.screenX,
+      screenY: e.screenY,
+      clickHideFilters: clickHideFilters
+    });
 
   // Highlight the elements specified by selector in yellow
   if (selectorList.length > 0)
     highlightElements(selectorList.join(","));
   // Now, actually highlight the element the user clicked on in red
   highlightElement(currentElement, "#fd1708", "#f6a1b5");
 
   // Make sure the browser doesn't handle this click
   e.preventDefault();
   e.stopPropagation();
@@ skipping 31 matching lines @@
 // In Chrome 37-40, the document_end content script (this one) runs properly, while the
 // document_start content scripts (that defines ext) might not. Check whether variable ext
 // exists before continuing to avoid "Uncaught ReferenceError: ext is not defined".
 // See https://crbug.com/416907
 if ("ext" in window && document instanceof HTMLDocument)
 {
   // Use a contextmenu handler to save the last element the user right-clicked on.
   // To make things easier, we actually save the DOM event.
   // We have to do this because the contextMenu API only provides a URL, not the actual
   // DOM element.
-  document.addEventListener('contextmenu', function(e) {
+  document.addEventListener('contextmenu', function(e)
+  {
     lastRightClickEvent = e;
+    // We also need to ensure any old lastRightClickEvent variables in other
+    // frames are cleared. So we store a random number, send a message to all
+    // frames that they should clear lastRightClickEvent unless the number matches
+    lastRightClickEventRandom = Math.random();

[Sebastian Noack, 2015/01/16 22:45:31]

I don't really like the approach here to generate a random number for each frame
to distinguish them. A random number isn't supposed to be a unique identifier.

I don't think you need to distinguish frames in the first place, but can just
protect the variable by a flag:

  lastRightClickEvent = e;
  lastRightClickEventValid = true;
  ext.backgroundPage.sendMessage({type: "clickhide-forget-contextmenu"});

  ...

  case "clickhide-forget-contextmenu":
    if (lastRightClickEventValid)
      lastRightClickEventValid = false;
    else
      lastRightClickEvent = null;

[kzar, 2015/01/16 23:03:45]

Done.

+    ext.backgroundPage.sendMessage(
+    {
+      type: "clickHide-clear-lastRightClickEvent",
+      random: lastRightClickEventRandom
+    });
   }, true);
 
   document.addEventListener("click", function(event)
   {
     // Ignore right-clicks
     if (event.button == 2)
       return;
 
     // Search the link associated with the click
     var link = event.target;
@@ skipping 82 matching lines @@
         }
         break;
       case "clickhide-move":
         if (clickHideFiltersDialog)
         {
           clickHideFiltersDialog.style.left = (parseInt(clickHideFiltersDialog.style.left, 10) + msg.x) + "px";
           clickHideFiltersDialog.style.top = (parseInt(clickHideFiltersDialog.style.top, 10) + msg.y) + "px";
         }
         break;
       case "clickhide-close":
-        if (clickHideFiltersDialog && msg.remove)
+        if (currentElement && msg.remove)
         {
           // Explicitly get rid of currentElement
           var element = currentElement.prisoner || currentElement;
           if (element && element.parentNode)
             element.parentNode.removeChild(element);
         }
         clickHide_deactivate();
         break;
+      case "clickHide-showDialog":

[Sebastian Noack, 2015/01/16 22:45:31]

For consistency with the message types implemented above, use dashes instead
camelcase here, and write "clickhide" lowercase as one word, same below.

[kzar, 2015/01/16 23:03:45]

Done.

+        if (window.self == window.top)
+          clickHide_showDialog(msg.screenX + window.pageXOffset,
+                               msg.screenY + window.pageYOffset,
+                               msg.clickHideFilters);
+        break;
+      case "clickHide-clear-lastRightClickEvent":
+        if (msg.random != lastRightClickEventRandom)
+          lastRightClickEvent = null;
+        lastRightClickEventRandom = null;
+        break;
     }
   });
 
   if (window == window.top)
     ext.backgroundPage.sendMessage({type: "report-html-page"});
 }