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;
+var lastRightClickEventValid = false;
 
 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 432 matching lines @@
 // popup asks for them.
 function clickHide_mouseClick(e)
 {
   if (!currentElement || !clickHide_activated)
     return;
 
   var elt = currentElement;
   if (currentElement.classList.contains("__adblockplus__overlay"))
     elt = currentElement.prisoner;
 
-  var clickHideFilters = new Array();
-  var selectorList = new Array();
+  var clickHideFilters = [];
+  var selectorList = [];
 
   var addSelector = function(selector)
   {
     if (selectorList.indexOf(selector) != -1)
       return;
 
     clickHideFilters.push(document.domain + "##" + selector);
     selectorList.push(selector);
   };
 
@@ skipping 36 matching lines @@
     if (style)
       addSelector(escapeCSS(elt.localName) + '[style=' + quote(style) + ']');
   }
 
   // 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
+      type: "forward",
+      payload:
+      {
+        type: "clickhide-show-dialog",
+        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();
@@ skipping 36 matching lines @@
 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)
   {
     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.

+    // frames are cleared.
+    lastRightClickEventValid = true;
     ext.backgroundPage.sendMessage(
     {
-      type: "clickHide-clear-lastRightClickEvent",
-      random: lastRightClickEventRandom
+      type: "forward",
+      payload:
+      {
+        type: "clickhide-clear-last-right-click-event"
+      }
     });
   }, true);
 
   document.addEventListener("click", function(event)
   {
     // Ignore right-clicks
     if (event.button == 2)
       return;
 
     // Search the link associated with the click
@@ skipping 92 matching lines @@
       case "clickhide-close":
         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.

+      case "clickhide-show-dialog":
         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)
+      case "clickhide-clear-last-right-click-event":
+        if (lastRightClickEventValid)
+          lastRightClickEventValid = false;
+        else
           lastRightClickEvent = null;
-        lastRightClickEventRandom = null;
         break;
     }
   });
 
   if (window == window.top)
     ext.backgroundPage.sendMessage({type: "report-html-page"});
 }