Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Merge pull request #14769 from brave/fix/14710-alt-shortcut
Browse files Browse the repository at this point in the history
Disable menu keyboard shortcut hint for 'Open new tor tab' menu item on Windows and Linux, only in certain menus.
  • Loading branch information
petemill authored Jul 18, 2018
2 parents c348daf + ebf4c1e commit 72f6c28
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 5 deletions.
4 changes: 3 additions & 1 deletion app/browser/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,14 @@ let appMenu = null
let closedFrames = new Immutable.OrderedMap()
let currentLocation = null

const mainMenuIsOSDrawn = isLinux || isDarwin

// Submenu initialization
const createFileSubmenu = () => {
const submenu = [
CommonMenu.newTabMenuItem(),
CommonMenu.newPrivateTabMenuItem(),
CommonMenu.newTorTabMenuItem(),
CommonMenu.newTorTabMenuItem(mainMenuIsOSDrawn),
CommonMenu.newPartitionedTabMenuItem(),
CommonMenu.newWindowMenuItem(),
CommonMenu.separatorMenuItem,
Expand Down
16 changes: 13 additions & 3 deletions app/common/commonMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,14 @@ const locale = require('../../js/l10n')
const settings = require('../../js/constants/settings')
const {tabs} = require('../../js/constants/config')
const {getSetting} = require('../../js/settings')
const platformUtil = require('./lib/platformUtil')
const communityURL = 'https://community.brave.com/'
const isDarwin = process.platform === 'darwin'
const electron = require('electron')
const menuUtil = require('./lib/menuUtil')

const isDarwin = platformUtil.isDarwin()
const isLinux = platformUtil.isLinux()

const ensureAtLeastOneWindow = (frameOpts) => {
// Handle no new tab requested, but need a window
// and possibly there is no window.
Expand Down Expand Up @@ -85,10 +88,17 @@ module.exports.newPrivateTabMenuItem = () => {
}
}

module.exports.newTorTabMenuItem = () => {
module.exports.newTorTabMenuItem = (isOSDrawn = true) => {
// On Windows and (often) Linux, a menu drawn by the OS
// via Muon will not display the 'Alt' modifier, so
// make sure we do not display a hint that is incorrect.
// On Windows we can leave it in sometimes since the main app menu is
// not OS-drawn, so we are ok to display the accelerator for it.
// On Linux, never display since it will be disabled in the OS-drawn App Menu.
const shouldShowAccelerator = !isLinux && (!isOSDrawn || isDarwin)
return {
label: locale.translation('newTorTab'),
accelerator: 'CmdOrCtrl+Alt+N',
accelerator: shouldShowAccelerator ? 'CmdOrCtrl+Alt+N' : undefined,
click: function (item, focusedWindow) {
ensureAtLeastOneWindow({
url: 'about:newtab',
Expand Down
2 changes: 1 addition & 1 deletion js/contextMenus.js
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,7 @@ function hamburgerTemplateInit (location, e) {
const template = [
CommonMenu.newTabMenuItem(),
CommonMenu.newPrivateTabMenuItem(),
CommonMenu.newTorTabMenuItem(),
CommonMenu.newTorTabMenuItem(false),
CommonMenu.newPartitionedTabMenuItem(),
CommonMenu.newWindowMenuItem(),
CommonMenu.separatorMenuItem,
Expand Down

0 comments on commit 72f6c28

Please sign in to comment.