diff --git a/lib/browser/init.ts b/lib/browser/init.ts index 47a3924d4b9..73418ac3d3e 100644 --- a/lib/browser/init.ts +++ b/lib/browser/init.ts @@ -195,13 +195,14 @@ app.on('window-all-closed', () => { } }) -Promise.all([ - import('@electron/internal/browser/default-menu'), - app.whenReady() -]).then(([{ setDefaultApplicationMenu }]) => { - // Create default menu - setDefaultApplicationMenu() -}) +const { setDefaultApplicationMenu } = require('@electron/internal/browser/default-menu') + +// Create default menu. +// +// Note that the task must be added before loading any app, so we can make sure +// the call is maded before any user window is created, otherwise the default +// menu may show even when user explicitly hides the menu. +app.once('ready', setDefaultApplicationMenu) if (packagePath) { // Finally load app's main.js and transfer control to C++. diff --git a/shell/browser/native_window_views.cc b/shell/browser/native_window_views.cc index dc84fefbffb..81a7558c4ab 100644 --- a/shell/browser/native_window_views.cc +++ b/shell/browser/native_window_views.cc @@ -1002,20 +1002,22 @@ void NativeWindowViews::SetFocusable(bool focusable) { void NativeWindowViews::SetMenu(AtomMenuModel* menu_model) { #if defined(USE_X11) - if (menu_model == nullptr) { + // Remove global menu bar. + if (global_menu_bar_ && menu_model == nullptr) { global_menu_bar_.reset(); root_view_->UnregisterAcceleratorsWithFocusManager(); return; } - if (!global_menu_bar_ && ShouldUseGlobalMenuBar()) - global_menu_bar_ = std::make_unique(this); - // Use global application menu bar when possible. - if (global_menu_bar_ && global_menu_bar_->IsServerStarted()) { - root_view_->RegisterAcceleratorsWithFocusManager(menu_model); - global_menu_bar_->SetMenu(menu_model); - return; + if (ShouldUseGlobalMenuBar()) { + if (!global_menu_bar_) + global_menu_bar_ = std::make_unique(this); + if (global_menu_bar_->IsServerStarted()) { + root_view_->RegisterAcceleratorsWithFocusManager(menu_model); + global_menu_bar_->SetMenu(menu_model); + return; + } } #endif diff --git a/spec-main/api-menu-spec.ts b/spec-main/api-menu-spec.ts index 98ff5f4b1ae..73e93816572 100644 --- a/spec-main/api-menu-spec.ts +++ b/spec-main/api-menu-spec.ts @@ -1,8 +1,14 @@ +import * as cp from 'child_process' +import * as path from 'path' import { expect } from 'chai' import { BrowserWindow, Menu, MenuItem } from 'electron' import { sortMenuItems } from '../lib/browser/api/menu-utils' +import { emittedOnce } from './events-helpers' +import { ifit } from './spec-helpers' import { closeWindow } from './window-helpers' +const fixturesPath = path.resolve(__dirname, 'fixtures') + describe('Menu module', function () { this.timeout(5000) describe('Menu.buildFromTemplate', () => { @@ -864,5 +870,27 @@ describe('Menu module', function () { Menu.setApplicationMenu(null) expect(Menu.getApplicationMenu()).to.be.null('application menu') }) + + ifit(process.platform !== 'darwin')('does not override menu visibility on startup', async () => { + const appPath = path.join(fixturesPath, 'api', 'test-menu-visibility') + const appProcess = cp.spawn(process.execPath, [appPath]) + + let output = '' + appProcess.stdout.on('data', data => { output += data }) + + await emittedOnce(appProcess, 'close') + expect(output).to.include('Window has no menu') + }) + + ifit(process.platform !== 'darwin')('does not override null menu on startup', async () => { + const appPath = path.join(fixturesPath, 'api', 'test-menu-null') + const appProcess = cp.spawn(process.execPath, [appPath]) + + let output = '' + appProcess.stdout.on('data', data => { output += data }) + + await emittedOnce(appProcess, 'close') + expect(output).to.include('Window has no menu') + }) }) }) diff --git a/spec-main/fixtures/api/test-menu-null/main.js b/spec-main/fixtures/api/test-menu-null/main.js new file mode 100644 index 00000000000..f4dd066d672 --- /dev/null +++ b/spec-main/fixtures/api/test-menu-null/main.js @@ -0,0 +1,17 @@ +const { app, BrowserWindow } = require('electron') + +let win +app.on('ready', function () { + win = new BrowserWindow({}) + win.loadURL('about:blank') + win.setMenu(null) + + setTimeout(() => { + if (win.isMenuBarVisible()) { + console.log('Window has a menu') + } else { + console.log('Window has no menu') + } + app.quit() + }) +}) diff --git a/spec-main/fixtures/api/test-menu-null/package.json b/spec-main/fixtures/api/test-menu-null/package.json new file mode 100644 index 00000000000..bfb7944df5c --- /dev/null +++ b/spec-main/fixtures/api/test-menu-null/package.json @@ -0,0 +1,4 @@ +{ + "name": "electron-test-menu", + "main": "main.js" +} diff --git a/spec-main/fixtures/api/test-menu-visibility/main.js b/spec-main/fixtures/api/test-menu-visibility/main.js new file mode 100644 index 00000000000..604dfe1ad91 --- /dev/null +++ b/spec-main/fixtures/api/test-menu-visibility/main.js @@ -0,0 +1,17 @@ +const { app, BrowserWindow } = require('electron') + +let win +app.on('ready', function () { + win = new BrowserWindow({}) + win.loadURL('about:blank') + win.setMenuBarVisibility(false) + + setTimeout(() => { + if (win.isMenuBarVisible()) { + console.log('Window has a menu') + } else { + console.log('Window has no menu') + } + app.quit() + }) +}) diff --git a/spec-main/fixtures/api/test-menu-visibility/package.json b/spec-main/fixtures/api/test-menu-visibility/package.json new file mode 100644 index 00000000000..bfb7944df5c --- /dev/null +++ b/spec-main/fixtures/api/test-menu-visibility/package.json @@ -0,0 +1,4 @@ +{ + "name": "electron-test-menu", + "main": "main.js" +}