Skip to content
This repository has been archived by the owner on Feb 26, 2022. It is now read-only.

Bug 836318: Items in submenus default to visible rather than using the PageContext #751

Merged
merged 1 commit into from Jan 31, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 3 additions & 8 deletions doc/module-source/sdk/context-menu.md
Expand Up @@ -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
-------------------

Expand All @@ -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
Expand Down
13 changes: 7 additions & 6 deletions lib/sdk/context-menu.js
Expand Up @@ -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))
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
37 changes: 37 additions & 0 deletions test/test-context-menu.js
Expand Up @@ -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();
Expand Down