refactor: return null when passing empty menu templates (#23364)

This commit is contained in:
shelley vohr 2020-05-04 08:19:21 -07:00 committed by GitHub
parent 59c1c12e0b
commit 0f0cc51b35
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 11 additions and 13 deletions

View file

@ -16,7 +16,7 @@ const MenuItem = function (options) {
} }
this.submenu = this.submenu || roles.getDefaultSubmenu(this.role); this.submenu = this.submenu || roles.getDefaultSubmenu(this.role);
if (this.submenu != null && this.submenu.constructor !== Menu) { if (this.submenu != null && this.submenu.constructor !== Menu) {
this.submenu = Menu.buildFromTemplate(this.submenu, true); this.submenu = Menu.buildFromTemplate(this.submenu);
} }
if (this.type == null && this.submenu != null) { if (this.type == null && this.submenu != null) {
this.type = 'submenu'; this.type = 'submenu';

View file

@ -173,11 +173,9 @@ Menu.setApplicationMenu = function (menu) {
} }
}; };
Menu.buildFromTemplate = function (template, isSubmenu = false) { Menu.buildFromTemplate = function (template) {
if (!Array.isArray(template)) { if (!Array.isArray(template)) {
throw new TypeError('Invalid template for Menu: Menu template must be an array'); throw new TypeError('Invalid template for Menu: Menu template must be an array');
} else if (!isSubmenu && template.length === 0) {
throw new TypeError('Invalid template for Menu: Menu template must be an non-empty array');
} }
if (!areValidTemplateItems(template)) { if (!areValidTemplateItems(template)) {

View file

@ -689,10 +689,16 @@ void TopLevelWindow::SetMenu(v8::Isolate* isolate, v8::Local<v8::Value> value) {
if (value->IsObject() && value->ToObject(context).ToLocal(&object) && if (value->IsObject() && value->ToObject(context).ToLocal(&object) &&
gin::ConvertFromV8(isolate, value, &menu) && !menu.IsEmpty()) { gin::ConvertFromV8(isolate, value, &menu) && !menu.IsEmpty()) {
menu_.Reset(isolate, menu.ToV8()); menu_.Reset(isolate, menu.ToV8());
window_->SetMenu(menu->model());
// We only want to update the menu if the menu has a non-zero item count,
// or we risk crashes.
if (menu->model()->GetItemCount() == 0) {
RemoveMenu();
} else {
window_->SetMenu(menu->model());
}
} else if (value->IsNull()) { } else if (value->IsNull()) {
menu_.Reset(); RemoveMenu();
window_->SetMenu(nullptr);
} else { } else {
isolate->ThrowException( isolate->ThrowException(
v8::Exception::TypeError(gin::StringToV8(isolate, "Invalid Menu"))); v8::Exception::TypeError(gin::StringToV8(isolate, "Invalid Menu")));

View file

@ -93,12 +93,6 @@ describe('Menu module', function () {
}).to.throw(/Invalid template for MenuItem: must have at least one of label, role or type/); }).to.throw(/Invalid template for MenuItem: must have at least one of label, role or type/);
}); });
it('throws when an empty array is passed as a template', () => {
expect(() => {
Menu.buildFromTemplate([]);
}).to.throw(/Invalid template for Menu: Menu template must be an non-empty array/);
});
it('throws when an non-array is passed as a template', () => { it('throws when an non-array is passed as a template', () => {
expect(() => { expect(() => {
Menu.buildFromTemplate('hello' as any); Menu.buildFromTemplate('hello' as any);