Skip to content

Commit

Permalink
Bug 832401: Put the new context-menu separator before the old one.
Browse files Browse the repository at this point in the history
(cherry picked from commit 990a49a)
(cherry picked from commit 74bc460)
  • Loading branch information
Mossop authored and Wes Kocher committed Jan 31, 2013
1 parent ae9f6dc commit 1836472
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/sdk/context-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,13 @@ let MenuWrapper = Class({
if (!this.separator) {
let separator = this.window.document.createElement("menuseparator");
separator.setAttribute("class", SEPARATOR_CLASS);
this.contextMenu.appendChild(separator);

// Insert before the separator created by the old context-menu if it
// exists to avoid bug 832401
let oldSeparator = this.window.document.getElementById("jetpack-context-menu-separator");
if (oldSeparator && oldSeparator.parentNode != this.contextMenu)
oldSeparator = null;
this.contextMenu.insertBefore(separator, oldSeparator);
}

let type = "menuitem";
Expand Down
22 changes: 22 additions & 0 deletions test/test-context-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,28 @@ const OVERFLOW_POPUP_CLASS = "addon-content-menu-overflow-popup";

const TEST_DOC_URL = module.uri.replace(/\.js$/, ".html");

// Tests that when present the separator is placed before the separator from
// the old context-menu module
exports.testSeparatorPosition = function (test) {
test = new TestHelper(test);
let loader = test.newLoader();

// Create the old separator
let oldSeparator = test.contextMenuPopup.ownerDocument.createElement("menuseparator");
oldSeparator.id = "jetpack-context-menu-separator";
test.contextMenuPopup.appendChild(oldSeparator);

// Create an item.
let item = new loader.cm.Item({ label: "item" });

test.showMenu(null, function (popup) {
test.assertEqual(test.contextMenuSeparator.nextSibling.nextSibling, oldSeparator,
"New separator should appear before the old one");
test.contextMenuPopup.removeChild(oldSeparator);
test.done();
});
};

// Destroying items that were previously created should cause them to be absent
// from the menu.
exports.testConstructDestroy = function (test) {
Expand Down

0 comments on commit 1836472

Please sign in to comment.