Issue 3223 - Make context menu e10s compatible in Firefox

lib/ui.js

Index: lib/ui.js
===================================================================
--- a/lib/ui.js
+++ b/lib/ui.js
@@ -1594,106 +1594,48 @@ let UI = exports.UI =
   },
 
   /**
    * Adds Adblock Plus menu items to the content area context menu when it shows
    * up.
    */
   fillContentContextMenu: function(/**Element*/ popup)
   {
-    let target = popup.triggerNode;
-    if (target instanceof Ci.nsIDOMHTMLMapElement || target instanceof Ci.nsIDOMHTMLAreaElement)
+    let window = popup.ownerDocument.defaultView;
+    if (!window.gContextMenuContentData ||
+      typeof window.gContextMenuContentData.addonInfo != "object" ||
+      typeof window.gContextMenuContentData.addonInfo.adblockplus != "object")
     {
-      // HTML image maps will usually receive events when the mouse pointer is
-      // over a different element, get the real event target.
-      let rect = target.getClientRects()[0];
-      target = target.ownerDocument.elementFromPoint(Math.max(rect.left, 0), Math.max(rect.top, 0));
+      return;
     }
 
-    if (!target)
-      return;
+    let items = window.gContextMenuContentData.addonInfo.adblockplus;
+    let clicked = null;
+    let menuItems = [];
 
-    let window = popup.ownerDocument.defaultView;
-    let menuItems = [];
-    let addMenuItem = function([node, nodeData])
+    function menuItemTriggered(id, nodeData)
+    {
+      clicked = id;
+      this.blockItem(window, id, nodeData);
+    }
+
+    for (let [id, nodeData] of items)
     {
       let type = nodeData.type.toLowerCase();
-      if (type == "background")
-      {
-        type = "image";
-        node = null;
-      }
-
       let label = this.overlay.attributes[type + "contextlabel"];
       if (!label)
         return;
 
       let item = popup.ownerDocument.createElement("menuitem");
       item.setAttribute("label", label);
       item.setAttribute("class", "abp-contextmenuitem");
-      item.addEventListener("command", this.blockItem.bind(this, window, node, nodeData), false);
+      item.addEventListener("command", menuItemTriggered.bind(this, id, nodeData), false);
       popup.appendChild(item);
 
       menuItems.push(item);
-    }.bind(this);
-
-    // Look up data that we have for the node
-    let data = RequestNotifier.getDataForNode(target);
-    let hadImage = false;
-    if (data && !data[1].filter)
-    {
-      addMenuItem(data);
-      hadImage = (data[1].type == "IMAGE");
-    }
-
-    // Look for frame data
-    let wnd = Utils.getWindow(target);
-    if (wnd.frameElement)
-    {
-      let data = RequestNotifier.getDataForNode(wnd.frameElement, true);
-      if (data && !data[1].filter)
-        addMenuItem(data);
-    }
-
-    // Look for a background image
-    if (!hadImage)
-    {
-      let extractImageURL = function(computedStyle, property)
-      {
-        let value = computedStyle.getPropertyCSSValue(property);
-        // CSSValueList
-        if ("length" in value && value.length >= 1)
-          value = value[0];
-        // CSSValuePrimitiveType
-        if ("primitiveType" in value && value.primitiveType == value.CSS_URI)
-          return Utils.unwrapURL(value.getStringValue()).spec;
-
-        return null;
-      };
-
-      let node = target;
-      while (node)
-      {
-        if (node.nodeType == Ci.nsIDOMNode.ELEMENT_NODE)
-        {
-          let style = wnd.getComputedStyle(node, "");
-          let bgImage = extractImageURL(style, "background-image") || extractImageURL(style, "list-style-image");
-          if (bgImage)
-          {
-            let data = RequestNotifier.getDataForNode(wnd.document, true, "IMAGE", bgImage);
-            if (data && !data[1].filter)
-            {
-              addMenuItem(data);
-              break;
-            }
-          }
-        }
-
-        node = node.parentNode;
-      }
     }
 
     // Add "Remove exception" menu item if necessary
     let location = this.getCurrentLocation(window);
     let filter = (location ? Policy.isWhitelisted(location.spec) : null);
     if (filter && filter.subscriptions.length && !filter.disabled)
     {
       let label = this.overlay.attributes.whitelistcontextlabel;
@@ -1705,30 +1647,31 @@ let UI = exports.UI =
       item.setAttribute("class", "abp-contextmenuitem");
       item.addEventListener("command", this.toggleFilter.bind(this, filter), false);
       popup.appendChild(item);
 
       menuItems.push(item);
     }
 
     // Make sure to clean up everything once the context menu is closed
-    if (menuItems.length)
+    let cleanUp = function(event)
     {
-      let cleanUp = function(event)
-      {
-        if (event.eventPhase != event.AT_TARGET)
-          return;
+      if (event.eventPhase != event.AT_TARGET)
+        return;
 
-        popup.removeEventListener("popuphidden", cleanUp, false);
-        for (let i = 0; i < menuItems.length; i++)
-          if (menuItems[i].parentNode)
-            menuItems[i].parentNode.removeChild(menuItems[i]);
-      }.bind(this);
-      popup.addEventListener("popuphidden", cleanUp, false);
-    }
+      popup.removeEventListener("popuphidden", cleanUp, false);
+      for (let menuItem of menuItems)
+        if (menuItem.parentNode)
+          menuItem.parentNode.removeChild(menuItem);
+
+      for (let [id, nodeData] of items)
+        if (id && id != clicked)
+          Policy.deleteNodes(id);

[Erik, 2015/12/03 23:21:57]

I'm not quite sure what this piece is for.

[Wladimir Palant, 2015/12/04 12:05:10]

We are keeping a reference to the nodes in the content process - that reference
needs to be released, otherwise we are going to leak memory. Only exception is
the menu entry that was clicked, the corresponding ID has to be passed to the
filter assistant, it will then call Policy.deleteNodes() when done.

The point of the exercise: providing immediate feedback when the user adds a
filter, by hiding the element that it applies to. Yes, users need to see that
Adblock Plus did something - and not merely after a reload.

+    }.bind(this);
+    popup.addEventListener("popuphidden", cleanUp, false);
   },
 
   /**
    * Called when the user presses a key in the application window, reacts to our
    * shortcut keys.
    */
   onKeyPress: function(/**Event*/ event)
   {