Conditional Creation of Context Menus

Add-ons Development
#1

OK, I am duplicating an existing legacy addon, and have the menu structure in a spreadsheet, along with the existing i18n locale data.

I can export the pertinent fields to a JSON file, which I save to disk in a sub-directory of the addon: (data/DefMenu.json, which is currently a subset of the 100+ menu items)

 [
  {
    "Navigation Menu": "TRUE",
    "menuId": "bbcwbx.bbcode",
    "argument": "",
    "menuTitle": "bbCode",
    "icons": null,
    "parentId": null
  },
  {
    "Navigation Menu": "TRUE",
    "menuId": "bbcwbx.bbcode.clp",
    "argument": "",
    "menuTitle": "Clipboard",
    "icons": null,
    "parentId": "bbcwbx.bbcode"
  },
  {
    "Navigation Menu": "FALSE",
    "menuId": "bbcwbx.bbcode.clp.quote",
    "argument": "[quote{{popup,\"Please insert author name or leave empty:\"}}]{{clipboard}}[/quote]",
    "menuTitle": "Quote",
    "icons": null,
    "parentId": "bbcwbx.bbcode.clp"
  },
  {
    "Navigation Menu": "FALSE",
    "menuId": "bbcwbx.bbcode.clp.code",
    "argument": "[code]{{clipboard}}[/quote]",
    "menuTitle": "Code",
    "icons": null,
    "parentId": "bbcwbx.bbcode.clp"
  },
  {
    "Navigation Menu": "FALSE",
    "menuId": "bbcwbx.bbcode.clp.list",
    "argument": "{{createList,{{clipboard}},\"bbcode\"}}",
    "menuTitle": "List",
    "icons": null,
    "parentId": "bbcwbx.bbcode.clp"
  }
]

Using this code, I can step through the individual records (rows): (sort of)

const defMenuURL = chrome.runtime.getURL('data/DefMenu.json');

fetch(defMenuURL)
  .then(function(response) {
    return response.json();
  })
  .then(function(defaultMenu) {
for(i = 0; i < defaultMenu.length; i++){
        browser.menus.create({
                    id: defaultMenu[i].menuId,
                    title: defaultMenu[i].menuTitle,  //eventually this becomes an i18n call
                    contexts: ["all"],
                    icons: defaultMenu[i].icons,
                    parentId: defaultMenu[i].parentId              
        }, console.log(defaultMenu[i].argument));

The problem is that some of the menus are top level menus, and hence have no parents, and some (most) of the menus have no icons, and when a blank or null argument is used, it throws an error.

Obviously, I could use a series of if then else statements with different browser.create.menu statements, but if I could do it inline, it would be cleaner and more readable.

I am looking for a syntax that is something like this:

browser.menus.create({
    id: defaultMenu[i].menuId,
    title: defaultMenu[i].menuTitle, //eventually this becomes an i18n call
    contexts: ["all"],
    if (defaultMenu[i].icons != "") {
        icons: defaultMenu[i].icons,
    }
    if (defaultMenu[i].parentId != "") {
        parentId: defaultMenu[i].parentId
    }
}, console.log(defaultMenu[i].argument));

Is this possible, or do I have to use if else or case statements for complete menu declarations?

#2

You can either use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Conditional_Operator to conditionally set the property value to what’s in the data structure or to something else (like undefined).

Alternatively you can create the object earlier and set the conditional properties inside the if.

Since your export sets the empty icons to null that should just work, no? Does the API enforce for the property to be unset then (so undefined)?

#3

More like this, I think:

for(i = 0; i < defaultMenu.length; i++){
    info = {
        id: defaultMenu[i].menuId,
        title: defaultMenu[i].menuTitle, //eventually this becomes an i18n call
        contexts: ["all"]
    };
    if (defaultMenu[i].icons != "") {
        info.icons = defaultMenu[i].icons;
    }
    if (defaultMenu[i].parentId != "") {
        info.parentId = defaultMenu[i].parentId;
    }
    browser.menus.create(
        info
    );
}
1 Like
#4

Thank you. That is insanely great.

Just checked it out, and it generated the whole context menu without a hitch.

#5

OK, I’ve also added the onclick parameter to the function, so the JSON now looks like this:

[
  {
    "menuId": "bbcwbx.bbcode.clp",
    "icons": "",
    "menuTitle": "Clipboard",
    "menuArg": "",
    "parentId": "bbcwbx.bbcode"
  },
  {
    "menuId": "bbcwbx.bbcode.clp.quote",
    "icons": "",
    "menuTitle": "Quote",
    "menuArg": "[quote{{popup}}]{{clipboard}}[/quote]",
    "parentId": "bbcwbx.bbcode.clp"
  },
]

So the menu program looks like this: (eventually invokeClick will send the argument to the content script for processing)

function invokeClick(zzarg){
console.log("Clicked on this: "+zzarg);
}

const defMenuURL = chrome.runtime.getURL('data/DefMenu.json');

fetch(defMenuURL)
    .then(function(response) {
        return response.json();
    })
    .then(function(defaultMenu) {
for(i = 0; i < defaultMenu.length; i++){
    info = {
        id: defaultMenu[i].menuId,
        title: defaultMenu[i].menuTitle, //eventually this becomes an i18n call
        contexts: ["all"]
    };
    if (defaultMenu[i].icons != "") {
        info.icons = defaultMenu[i].icons;
    }
    if (defaultMenu[i].parentId != "") {
        info.parentId = defaultMenu[i].parentId;
    }
    if (defaultMenu[i].menuArg != "") {
        info.onclick = function(){invokeClick(defaultMenu[i].menuArg)};
    }
    browser.menus.create(
        info
    );
}})

I get “defaultMenu[i] is undefined”.

Am I getting not using the onclick parameter properly?

From the docs:

onclick Optional

function . A function that will be called when the menu item is clicked. Event pages cannot use this: instead, they should register a listener for menus.onClicked .

#6

When clicking the menu item? That suggests the way the function was created the value did not get plugged for defaultMenu[i].menuArg as expected.

I used the other approach to handling clicks, which was to have a single event handler and branch on the menuItemId: https://github.com/jscher2000/switch-to-previous-active-tab/blob/master/background.js#L439

Since you have a data structure that presumably will still be available, that should be pretty efficient for you, I would think. Maybe: https://stackoverflow.com/questions/7364150/find-object-by-id-in-an-array-of-javascript-objects

#7

Thanks, I will check that out.

I did a little cod code swap to see what would happen:

I replaced the line

info.onclick = function(){invokeClick(defaultMenu[i].menuArg)};

with the line

info.onclick = function(){invokeClick(i)};

I do get a response from the console,

Clicked on this: 106

Where 106 is the total number of rows in the object, so it’s not finding the reference because there is none.

So it appears that the script is not pulling the proper menu (in this case, row 3 of the object) when I click.

So redoing the lookup inside the invokeClick function would seem to be the ticket.

#8

The real problem here is that the variables are not appropriately scoped. I considered to comment on this earlier but was still in the vain hope that it wouldn’t cause problems.

Its actually a bit more complicated, but every time you use a variable name, it addresses a “memory slot” (variable).

Variables can either be explicitly declared (let, const, function arguments) and will then be bound/restricted to that “scope” (basically the surrounding pair of curly brackets) or it will be implicitly created as a “global” variable.

In your code examples, the loop index i is not declared, meaning you always and forever refer to the same memory slot. After each loop iteration it is incremented, giving you the expected value for that pass, but at the end of the loop it is left at one index past the end of the array, and keeps that value forever (unless you happen to use i as a variable name in code that is executed later.
Therefore defaultMenu[i] does not give you what you want.

This demonstrates pretty well why using undeclared/global variables is a very, very, very bad practice, even in example code.
Always use let or const when you need a new place to store a value. JS is “clever” (garbage collection) and keeps them around only as long as necessary. SO you may be wasteful here.

You may also want to read about strict mode and then put a 'use strict'; at the top of every file, to avoid some of this crap.

At the same time, why do you even use i? Are you counting stuff? Listing soemthing by index? No! You are iterating over a collection of objects. So do that: for (const menu of defaultMenu) or defaultMenu.forEach(menu =>. (And I’d call it defaultMenus, since its a collection of multiple menus.)

And for readability (and some other reasons), (usually) don’t use function when it doesn’t need a name, but use => arrow functions.

In summary, this should work:

(async function(global) { // in case you ever actually want a global variable, refer to it as `global.<name>`, to make clear what you are doing
'use strict'; // enable strict mode

function invokeClick(zzarg) {
    console.log("Clicked on this: ", zzarg);
}

const defMenuURL = chrome.runtime.getURL('data/DefMenu.json');

const defaultMenus = (await (await fetch(defMenuURL)).json());

for (const menu of defaultMenus) {
    const info = {
        id: defaultMenu[i].menuId,
        title: defaultMenu[i].menuTitle, //eventually this becomes an i18n call
        contexts: ["all"]
    };
    if (defaultMenu[i].icons) { // don't compare ` != ""` unless you really know what it does and it does what you want. I guess you actually mean "if it is set", which `if (it)` already does
        info.icons = defaultMenu[i].icons;
    }
    if (defaultMenu[i].parentId) {
        info.parentId = defaultMenu[i].parentId;
    }
    if (defaultMenu[i].menuArg) {
        info.onclick = () => invokeClick(menu.menuArg);
    }
    browser.menus.create(
        info
    ); // be aware that you ignore whether this actually succeeds, which is generally not good
}

})(this);

I’d consider every single change in the code above to your last version essential. The version below actually also listens for errors:

(async function(global) { 'use strict';

function invokeClick(zzarg) {
    console.log("Clicked on this: ", zzarg);
}

const defMenuURL = chrome.runtime.getURL('data/DefMenu.json');

(await Promise.all((await (await fetch(defMenuURL)).json()).map(menu => {
    const info = {
        id: defaultMenu[i].menuId,
        title: defaultMenu[i].menuTitle, //eventually this becomes an i18n call
        contexts: ["all"]
    };
    if (defaultMenu[i].icons) {
        info.icons = defaultMenu[i].icons;
    }
    if (defaultMenu[i].parentId) {
        info.parentId = defaultMenu[i].parentId;
    }
    if (defaultMenu[i].menuArg) {
        info.onclick = () => invokeClick(menu.menuArg);
    }
    return browser.menus.create(
        info
    );
}))); // errors are "collected, but still just ignored here ...

})(this);