Start menu handler ids at one, not zero

[tsmaeder]

What it does

In order to handle native menu invocations on the browser side, we allocate handler ids when that we send over to the electron-main side. On the electron-main side, we check for the handler id with if (menu.handlerId) before invoking the menu action. Since the handler ids start at 0, the very first menu action created could never be invoked.

Fixes #14279

Contributed on behalf of STMicroelectronics

How to test

This can't really be tested in our example applications. But I think I can convince a reviewer that the change at least does no harm: we just need to make sure menus and context menus still work as advertised.

Follow-ups

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[msujew]

Looks good, however, regarding:

On the electron-main side, we check for the handler id with if (menu.handlerId) before invoking the menu action.

Does it make sense to rewrite this to if (typeof menu.handlerId === 'number')? We also do this in the plugin system to ensure that 0 is a valid value.

[tsmaeder]

Looks good, however, regarding:

On the electron-main side, we check for the handler id with if (menu.handlerId) before invoking the menu action.

Does it make sense to rewrite this to if (typeof menu.handlerId === 'number')? We also do this in the plugin system to ensure that 0 is a valid value.

While it is the better check in this case, I think it's better to fix this problem at the source, i.e. the generation of the id's in general. Here's why: the client does not have to care whether the id is a number or a string or undefined: the empty string might well be the in indication of "no id" and is often not considered a valid id. It is hard to make sure that we do the check correctly for the type of id in all places, but if we ensure valid id values are never falsy, it just doesn't matter how we check for it. It's more robust as a policy, IMO.