diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md index 06d9cf4f9380..ae04a286821d 100644 --- a/docs/api/menu-item.md +++ b/docs/api/menu-item.md @@ -47,6 +47,9 @@ The built-in `role` behavior will give the best native experience. The `label` and `accelerator` values are optional when using a `role` and will default to appropriate values for each platform. +Every menu item must have either a `role`, `label`, or in the case of a separator +a `type`. + The `role` property can have following values: * `undo` diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index c0cc199e05a8..506e3525c10c 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -156,25 +156,28 @@ Menu.setApplicationMenu = function (menu) { Menu.buildFromTemplate = function (template) { if (!Array.isArray(template)) { - throw new TypeError('Invalid template for Menu') + throw new TypeError('Invalid template for Menu: Menu template must be an array') } - const menu = new Menu() + if (!areValidTemplateItems(template)) { + throw new TypeError('Invalid template for MenuItem: must have at least one of label, role or type') + } const filtered = removeExtraSeparators(template) const sorted = sortTemplate(filtered) - sorted.forEach((item) => { - if (typeof item !== 'object') { - throw new TypeError('Invalid template for MenuItem') - } - menu.append(new MenuItem(item)) - }) + sorted.forEach((item) => menu.append(new MenuItem(item))) return menu } /* Helper Functions */ +// validate the template against having the wrong attribute +function areValidTemplateItems (template) { + return template.every(item => + item != null && typeof item === 'object' && (item.hasOwnProperty('label') || item.hasOwnProperty('role') || item.type === 'separator')) +} + function sortTemplate (template) { const sorted = sortMenuItems(template) for (let id in sorted) { diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index e4566dba43a8..ed2749700c6c 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -42,6 +42,23 @@ describe('Menu module', () => { }).to.not.throw() }) + it('does throw exceptions for empty objects and null values', () => { + expect(() => { + Menu.buildFromTemplate([{}, null]) + }).to.throw(/Invalid template for MenuItem: must have at least one of label, role or type/) + }) + + it('does throw exception for object without role, label, or type attribute', () => { + expect(() => { + Menu.buildFromTemplate([{ 'visible': true }]) + }).to.throw(/Invalid template for MenuItem: must have at least one of label, role or type/) + }) + it('does throw exception for undefined', () => { + expect(() => { + Menu.buildFromTemplate([undefined]) + }).to.throw(/Invalid template for MenuItem: must have at least one of label, role or type/) + }) + describe('Menu sorting and building', () => { describe('sorts groups', () => { it('does a simple sort', () => {