Custom Menu API Crash Due to Localization

[Wy yw]

If an menu item has a l10nid and the plugin is disabled, the context menu will crash. This occurred on Zotero 9.0-beta.21 on MacOS, and it happens even if the .ftl file is never loaded.

The relevant code snippet is:

const menus = [
    {
        menuType: 'menuitem',
        l10nID: 'loc-me',
    }
];

Zotero.MenuManager.registerMenu({
    menuID: 'blah',
    pluginID: 'blah',
    target: 'main/library/item',
    menus: menus
});

The error console spits out:

Uncaught (in promise) undefined zoteroPane.js:3281:22

    onItemsContextMenuOpen chrome://zotero/content/zoteroPane.js:3281

    AsyncFunctionThrow self-hosted:804

    (Async: async)

    onContextMenu chrome://zotero/content/zoteroPane.js:1670

    onItemContextMenu chrome://zotero/content/itemTree.js:1177

    _handleContextMenu chrome://zotero/content/components/virtualized-table.js:749

    _renderItem chrome://zotero/content/components/virtualized-table.js:1131

​Thank you.

[Wy yw]

Actually, maybe a better code snippet will be:

const menus = [
    {
        menuType: 'menuitem',
        l10nID: 'loc-me',
    }
];

let a = Zotero.MenuManager.registerMenu({

    menuID: 'blah',
    pluginID: 'blah',
    target: 'main/library/item',
    menus: menus
});

Then open the context menu. Then try the snippet:

Zotero.MenuManager.unregisterMenu(a);

Thank you.

[Northword]

This should have been fixed in Zotero 10-beta. Perhaps you can try it in 10-beta to see if it works properly.

[Wy yw]

Hi,

I have tried the code snippet in Zotero 10.0-beta.2 and the issue still occurs.

Thank you.

[XY Wong]

I don't have further info about how you did with the FTL, but from my understanding, this is likely related to plugins not unregistering the FTL resources properly.

If you register FTL with `document.l10n.addResourceIds`, you need to unregister with `document.l10n.removeResourceIds`;

If you register with `MozXULElement.insertFTLIfNeeded`, you need to unregister with `document.querySelector([href={you-file}.ftl])?.remove()`.

[Wy yw]

Hi,

This is unrelated to the FTL loading/unloading. If you just run the code snippet in the Zotero console you can reproduce. It only occurs if you open the menu before unregistering the menu, so maybe it is a caching issue.

Thank you.

[Wy yw]

Maybe I should add that it doesn't seem to be directly caused by disabling the plugin, but rather unregistering the menu.

[XY Wong]

That doesn't make sense, as you are testing against an invalid FTL id, which will break the translateFragment call and thus break the menu.

[Wy yw]

Hi,

I think I have identified the problem.

In the buildItemContextMenu function, line 4242, Zotero_LocateMenu.buildContextMenu is called before Zotero.MenuManager.updateMenuPopup. The issue is that buildContextMenu calls translateFragment before updateMenuPopup removes the old menu item with the l10n, causing buildItemContextMenu to throw an exception, causing no context menu to pop up.

I can fix it by manually calling updateMenuPopup to correctly flush the context menu.

I am not too familiar with the code base, but I would prematurely think that switching the call order would fix the issue. Or just call updateMenuPopup when the plugin is disabled/deregistered.

Thank you.

[XY Wong]

The test you did with an invalid FTL won't work, because the invalid FTL id breaks the `translateFragment`, as I explained above. This is nothing to do with your original post that menus registered by plugins breaks the menu after it's disabled, which was already fixed in the recent beta and stable releases.

[Wy yw]

Hi,

For future readers, I have fixed the issue of the context menu crashing when the plugin is disabled by disabling my manual call of Zotero.MenuManager.unregisterMenu on plugin shutdown and instead allowed for automatic cleanup by Zotero. This crash happened even though the .ftl files were properly unloaded before calling unregisterMenu. It seems that the _unregisterByPluginID does some additional cleanup that manually calling unregisterMenu does not do.

Thank you.

[XY Wong]

To clarify, there are several different things being discussed here (and also in the forums like https://forums.zotero.org/discussion/comment/510987/):

1. Issue with plugin disabling, which is fixed;

2. Issue with plugin startup order affecting the menu (https://forums.zotero.org/discussion/comment/510987/#Comment_510987), which is likely from the ftl rust implementation's resource query logic. We will try to give a fix.

3. Issue with invalid ftl id breaking the menu. This isn't a meaningful test, as plugins should always make sure their FTL are correct otherwise the UI will anyway break here or there. Though, we will try our best to make it robust and prevent this from happening if possible.

As stated in the docs, plugins don't have to manually call unregister on disable.