fix: Stricter Testing For Menu Items (#13992)

This PR includes stricter testing for empty objects so that false context menus are not created along with the tests to ensure future compatibility.

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`

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) => {
+  sorted.forEach((item) => menu.append(new MenuItem(item)))
-    if (typeof item !== 'object') {
-      throw new TypeError('Invalid template for MenuItem')
-    }
-    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) {

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', () => {