From 5ea05ddee768893fb61ba63a1bfaf34807c22ba3 Mon Sep 17 00:00:00 2001
From: Troy <15041351+Kthulu120@users.noreply.github.com>
Date: Fri, 17 Aug 2018 15:10:14 -0500
Subject: [PATCH] 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   |  3 +++
 lib/browser/api/menu.js | 19 +++++++++++--------
 spec/api-menu-spec.js   | 17 +++++++++++++++++
 3 files changed, 31 insertions(+), 8 deletions(-)

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