Permalink
Browse files

Bug 836318: Items in submenus default to visible rather than using th…

…e PageContext.
  • Loading branch information...
1 parent e552732 commit 926d4fce9cda8c095028e6d9324b7dcc257f75bb @Mossop committed Jan 30, 2013
Showing with 47 additions and 14 deletions.
  1. +3 −8 doc/module-source/sdk/context-menu.md
  2. +7 −6 lib/sdk/context-menu.js
  3. +37 −0 test/test-context-menu.js
View
11 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
View
13 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;
@@ -967,7 +967,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;
View
37 test/test-context-menu.js
@@ -2031,6 +2031,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();

0 comments on commit 926d4fc

Please sign in to comment.