From 0651ece3d98bfdf6307f3ed1c020fe475ce1c50c Mon Sep 17 00:00:00 2001
From: Erik Vold <erikvvold@gmail.com>
Date: Wed, 30 Jan 2013 20:06:43 -0800
Subject: [PATCH] Merge pull request #751 from Mossop/bug836318

Bug 836318: Items in submenus default to visible rather than using the PageContext r=@erikvold(cherry picked from commit 8ecc5decb9cd85c8c8eebebbb1c7fba15b039930)
---
 doc/module-source/sdk/context-menu.md | 11 +++-----
 lib/sdk/context-menu.js               | 13 +++++-----
 test/test-context-menu.js             | 37 +++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/doc/module-source/sdk/context-menu.md b/doc/module-source/sdk/context-menu.md
index 79a4bc023..650ff66e4 100644
--- a/doc/module-source/sdk/context-menu.md
+++ b/doc/module-source/sdk/context-menu.md
@@ -39,12 +39,6 @@ configurable with the `extensions.addon-sdk.context-menu.overflowThreshold`
 preference) all of the menu items will instead appear in an overflow menu to
 avoid making the context menu too large.
 
-Note that *context menu items are only displayed when the page has finished loading*.
-While the page is still loading, or if you cancel load, context menu items
-added using this module will not appear.
-[Bug 714914](https://bugzilla.mozilla.org/show_bug.cgi?id=714914) tracks
-this problem.
-
 Specifying Contexts
 -------------------
 
@@ -59,8 +53,9 @@ or the node the user clicked to open the menu.
 
 ### The Page Context
 
-First of all, you may not need to specify a context at all.  When an item does
-not specify a context, the page context applies.
+First of all, you may not need to specify a context at all.  When a top-level
+item does not specify a context, the page context applies.  An item that is in a
+submenu is visible unless you specify a context.
 
 The *page context* occurs when the user invokes the context menu on a
 non-interactive portion of the page.  Try right-clicking a blank spot in this
diff --git a/lib/sdk/context-menu.js b/lib/sdk/context-menu.js
index c9110d0c7..6b2102501 100644
--- a/lib/sdk/context-menu.js
+++ b/lib/sdk/context-menu.js
@@ -335,11 +335,11 @@ function getCurrentWorkerContext(item, popupNode) {
 
 // Tests whether an item should be visible or not based on its contexts and
 // content scripts
-function isItemVisible(item, popupNode) {
+function isItemVisible(item, popupNode, defaultVisibility) {
   if (!item.context.length) {
     let worker = getItemWorkerForWindow(item, popupNode.ownerDocument.defaultView);
     if (!worker || !worker.anyContextListeners())
-      return PageContext().isCurrent(popupNode);
+      return defaultVisibility;
   }
 
   if (!hasMatchingContext(item.context, popupNode))
@@ -714,16 +714,16 @@ let MenuWrapper = Class({
 
   // Recurses through the menu setting the visibility of items. Returns true
   // if any of the items in this menu were visible
-  setVisibility: function setVisibility(menu, popupNode) {
+  setVisibility: function setVisibility(menu, popupNode, defaultVisibility) {
     let anyVisible = false;
 
     for (let item of internal(menu).children) {
-      let visible = isItemVisible(item, popupNode);
+      let visible = isItemVisible(item, popupNode, defaultVisibility);
 
       // Recurse through Menus, if none of the sub-items were visible then the
       // menu is hidden too.
       if (visible && (item instanceof Menu))
-        visible = this.setVisibility(item, popupNode);
+        visible = this.setVisibility(item, popupNode, true);
 
       let xulNode = this.getXULNodeForItem(item);
       xulNode.hidden = !visible;
@@ -973,7 +973,8 @@ let MenuWrapper = Class({
       let separator = this.separator;
       let popup = this.overflowMenu;
   
-      if (this.setVisibility(this.items, event.target.triggerNode)) {
+      let popupNode = event.target.triggerNode;
+      if (this.setVisibility(this.items, popupNode, PageContext().isCurrent(popupNode))) {
         // Some of this instance's items are visible so make sure the separator
         // and if necessary the overflow popup are visible
         separator.hidden = false;
diff --git a/test/test-context-menu.js b/test/test-context-menu.js
index 05a9c283f..ce6dc7df3 100644
--- a/test/test-context-menu.js
+++ b/test/test-context-menu.js
@@ -2053,6 +2053,43 @@ exports.testSubItemContextMatch = function (test) {
   });
 };
 
+
+// Child items should default to visible, not to PageContext
+exports.testSubItemDefaultVisible = function (test) {
+  test = new TestHelper(test);
+  let loader = test.newLoader();
+
+  let items = [
+    loader.cm.Menu({
+      label: "menu 1",
+      context: loader.cm.SelectorContext("img"),
+      items: [
+        loader.cm.Item({
+          label: "subitem 1"
+        }),
+        loader.cm.Item({
+          label: "subitem 2",
+          context: loader.cm.SelectorContext("img")
+        }),
+        loader.cm.Item({
+          label: "subitem 3",
+          context: loader.cm.SelectorContext("a")
+        })
+      ]
+    })
+  ];
+
+  // subitem 3 will be hidden
+  let hiddenItems = [items[0].items[2]];
+
+  test.withTestDoc(function (window, doc) {
+    test.showMenu(doc.getElementById("image"), function (popup) {
+      test.checkMenu(items, hiddenItems, []);
+      test.done();
+    });
+  });
+};
+
 exports.testSubItemClick = function (test) {
   test = new TestHelper(test);
   let loader = test.newLoader();