diff --git a/browser/components/extensions/ext-contextMenus.js b/browser/components/extensions/ext-contextMenus.js index f6d04e887ef6..6978451e0286 100644 --- a/browser/components/extensions/ext-contextMenus.js +++ b/browser/components/extensions/ext-contextMenus.js @@ -240,20 +240,33 @@ MenuItem.prototype = { return this._id; }, + ensureValidParentId(parentId) { + if (parentId === undefined) { + return; + } + let menuMap = contextMenuMap.get(this.extension); + if (!menuMap.has(parentId)) { + throw new Error("Could not find any MenuItem with id: " + parentId); + } + for (let item = menuMap.get(parentId); item; item = item.parent) { + if (item === this) { + throw new Error("MenuItem cannot be an ancestor (or self) of its new parent."); + } + } + }, + set parentId(parentId) { - // TODO: cycle check here + this.ensureValidParentId(parentId); + if (this.parent) { this.parent.detachChild(this); } - let menuMap = contextMenuMap.get(this.extension); - if (menuMap.has(parentId)) { + if (parentId === undefined) { + this.root.addChild(this); + } else { + let menuMap = contextMenuMap.get(this.extension); menuMap.get(parentId).addChild(this); - } else if (parentId !== undefined) { - // Unless the intention was to null the parentId, if we cannot find - // any item with the given id, we should throw. - let e = new Error("Could not find any MenuItem with id: " + parentId); - throw e; } }, diff --git a/browser/components/extensions/test/browser/browser_ext_contextMenus.js b/browser/components/extensions/test/browser/browser_ext_contextMenus.js index da6c8f23da6c..ba826d449925 100644 --- a/browser/components/extensions/test/browser/browser_ext_contextMenus.js +++ b/browser/components/extensions/test/browser/browser_ext_contextMenus.js @@ -54,7 +54,12 @@ add_task(function* () { { title: "child2", parentId: parentToDel, onclick: genericOnClick }); browser.contextMenus.remove(parentToDel); - browser.test.notifyPass(); + try { + browser.contextMenus.update(parent, { parentId: child2 }); + browser.test.notifyFail(); + } catch(e) { + browser.test.notifyPass(); + } }, });