Merge pull request #7498 from electron/custom-props-in-menu-item-constructor

Preserve custom properties passed to MenuItem constructor

lib/browser/api/menu-item.js

@@ -7,18 +7,11 @@ let nextCommandId = 0
 const MenuItem = function (options) {
   const {Menu} = require('electron')
 
-  this.selector = options.selector
+  // Preserve extra fields specified by user
-  this.type = options.type
+  for (let key in options) {
-  this.role = options.role
+    if (!(key in this)) this[key] = options[key]
-  this.label = options.label
+  }
-  this.sublabel = options.sublabel
-  this.accelerator = options.accelerator
-  this.icon = options.icon
-  this.enabled = options.enabled
-  this.visible = options.visible
-  this.checked = options.checked
 
-  this.submenu = options.submenu
   if (this.submenu != null && this.submenu.constructor !== Menu) {
     this.submenu = Menu.buildFromTemplate(this.submenu)
   }
@@ -70,10 +63,7 @@ MenuItem.prototype.getDefaultRoleAccelerator = function () {
   return roles.getDefaultAccelerator(this.role)
 }
 
-MenuItem.prototype.overrideProperty = function (name, defaultValue) {
+MenuItem.prototype.overrideProperty = function (name, defaultValue = null) {
-  if (defaultValue == null) {
-    defaultValue = null
-  }
   if (this[name] == null) {
     this[name] = defaultValue
   }

lib/browser/api/menu.js

@@ -272,7 +272,7 @@ Menu.getApplicationMenu = function () {
 Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder
 
 Menu.buildFromTemplate = function (template) {
-  var insertIndex, item, j, k, key, len, len1, menu, menuItem, positionedTemplate
+  var insertIndex, item, j, k, len, len1, menu, menuItem, positionedTemplate
   if (!Array.isArray(template)) {
     throw new TypeError('Invalid template for Menu')
   }
@@ -295,12 +295,6 @@ Menu.buildFromTemplate = function (template) {
       throw new TypeError('Invalid template for MenuItem')
     }
     menuItem = new MenuItem(item)
-    for (key in item) {
-      // Preserve extra fields specified by user
-      if (!menuItem.hasOwnProperty(key)) {
-        menuItem[key] = item[key]
-      }
-    }
     menu.append(menuItem)
   }
   return menu

spec/api-menu-spec.js

@@ -430,3 +430,25 @@ describe('menu module', function () {
     })
   })
 })
+
+describe('MenuItem with custom properties in constructor', function () {
+  it('preserves the custom properties', function () {
+    var template = [{
+      label: 'menu 1',
+      customProp: 'foo',
+      submenu: []
+    }]
+
+    var menu = Menu.buildFromTemplate(template)
+    menu.items[0].submenu.append(new MenuItem({
+      label: 'item 1',
+      customProp: 'bar',
+      overrideProperty: 'oops not allowed'
+    }))
+
+    assert.equal(menu.items[0].customProp, 'foo')
+    assert.equal(menu.items[0].submenu.items[0].label, 'item 1')
+    assert.equal(menu.items[0].submenu.items[0].customProp, 'bar')
+    assert.equal(typeof menu.items[0].submenu.items[0].overrideProperty, 'function')
+  })
+})