Bug 1215376 - Ensure that menu elements have a unique ID
Previously, there was no guarantee for the uniqueness of the ID.
Now the ID is unique, and always set.
See https://bugzil.la/1358213#c6 and #c7 for examples of collisions.
MozReview-Commit-ID: 7CHP6tWFDUg
--- a/browser/components/extensions/ext-menus.js
+++ b/browser/components/extensions/ext-menus.js
@@ -194,20 +194,17 @@ var gMenuBuilder = {
selection = selection.substring(0, maxSelectionLength - 3) + "...";
}
label = label.replace(/%s/g, selection);
}
element.setAttribute("label", label);
}
- if (item.id && item.extension && item.extension.id) {
- element.setAttribute("id",
- `${makeWidgetId(item.extension.id)}_${item.id}`);
- }
+ element.setAttribute("id", item.elementId);
if (item.icons) {
this.setMenuItemIcon(element, item.extension, contextData, item.icons);
}
if (item.type == "checkbox") {
element.setAttribute("type", "checkbox");
if (item.checked) {
@@ -441,16 +438,28 @@ MenuItem.prototype = {
}
this._id = id;
},
get id() {
return this._id;
},
+ get elementId() {
+ let id = this.id;
+ // If the ID is an integer, it is auto-generated and globally unique.
+ // If the ID is a string, it is only unique within one extension and the
+ // ID needs to be concatenated with the extension ID.
+ if (typeof id != "number") {
+ // To avoid collisions with numeric IDs, add a prefix to string IDs.
+ id = `_${id}`;
+ }
+ return `${makeWidgetId(this.extension.id)}-menuitem-${id}`;
+ },
+
ensureValidParentId(parentId) {
if (parentId === undefined) {
return;
}
let menuMap = gMenuMap.get(this.extension);
if (!menuMap.has(parentId)) {
throw new Error("Could not find any MenuItem with id: " + parentId);
}
--- a/browser/components/extensions/test/browser/browser_ext_menus.js
+++ b/browser/components/extensions/test/browser/browser_ext_menus.js
@@ -74,17 +74,17 @@ add_task(async function test_actionConte
is(submenu.tagName, "menu", "Correct submenu type");
is(submenu.label, "parent", "Correct submenu title");
const popup = await openSubmenu(submenu);
is(popup, submenu.firstChild, "Correct submenu opened");
is(popup.children.length, 2, "Correct number of submenu items");
- let idPrefix = `${makeWidgetId(extension.id)}_`;
+ let idPrefix = `${makeWidgetId(extension.id)}-menuitem-_`;
is(second.tagName, "menuitem", "Second menu item type is correct");
is(second.label, "click 1", "Second menu item title is correct");
is(second.id, `${idPrefix}1`, "Second menu item id is correct");
is(last.label, "click 5", "Last menu item title is correct");
is(last.id, `${idPrefix}5`, "Last menu item id is correct");
is(separator.tagName, "menuseparator", "Separator after last menu item");