diff --git a/app/browser/lib/menuUtil.js b/app/browser/lib/menuUtil.js index 45cc12976b4..c8b6b55be2e 100644 --- a/app/browser/lib/menuUtil.js +++ b/app/browser/lib/menuUtil.js @@ -10,9 +10,6 @@ const eventUtil = require('../../../js/lib/eventUtil') const siteUtil = require('../../../js/state/siteUtil') const locale = require('../../locale') -// States which can trigger dynamic menus to change -let lastSettingsState, lastSites, lastClosedFrames - /** * Get an electron MenuItem object for the PARENT menu (File, Edit, etc) based on its label * @param {string} label - the text associated with the menu @@ -55,45 +52,6 @@ module.exports.getMenuItem = (appMenu, label) => { return null } -/** - * Check for uneeded updates. - * Updating the menu when it is not needed causes the menu to close if expanded - * and also causes menu clicks to not work. So we don't want to update it a lot. - * Should only be updated when appState or windowState change (for history or bookmarks) - * NOTE: settingsState is not used directly; it gets used indirectly via getSetting() - * @param {Object} appState - Application state. Used to fetch bookmarks and settings (like homepage) - * @param {Object} windowData - Information specific to the current window (recently closed tabs, etc) - */ -module.exports.checkForUpdate = (appState, windowData) => { - const updated = { - nothing: true, - settings: false, - sites: false, - closedFrames: false - } - - if (appState && appState.get('settings') !== lastSettingsState) { - // Currently only used for the HOMEPAGE value (bound to history menu) - lastSettingsState = appState.get('settings') - updated.nothing = false - updated.settings = true - } - - if (appState && appState.get('sites') !== lastSites) { - lastSites = appState.get('sites') - updated.nothing = false - updated.sites = true - } - - if (windowData && windowData.get('closedFrames') !== lastClosedFrames) { - lastClosedFrames = windowData.get('closedFrames') - updated.nothing = false - updated.closedFrames = true - } - - return updated -} - const createBookmarkMenuItems = (bookmarks, parentFolderId) => { const filteredBookmarks = parentFolderId ? bookmarks.filter((bookmark) => bookmark.get('parentFolderId') === parentFolderId) @@ -131,14 +89,11 @@ const createBookmarkMenuItems = (bookmarks, parentFolderId) => { return payload } -module.exports.createBookmarkMenuItems = () => { - if (lastSites) { - return createBookmarkMenuItems(siteUtil.getBookmarks(lastSites)) - } - return [] +module.exports.createBookmarkMenuItems = (sites) => { + return createBookmarkMenuItems(siteUtil.getBookmarks(sites)) } -module.exports.createRecentlyClosedMenuItems = () => { +module.exports.createRecentlyClosedMenuItems = (lastClosedFrames) => { const payload = [] if (lastClosedFrames && lastClosedFrames.size > 0) { payload.push( diff --git a/app/browser/menu.js b/app/browser/menu.js index 4101c0e060d..93847dc73ca 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -7,12 +7,17 @@ const Immutable = require('immutable') const electron = require('electron') const appConfig = require('../../js/constants/appConfig') +const appActions = require('../../js/actions/appActions') +const appConstants = require('../../js/constants/appConstants') +const appDispatcher = require('../../js/dispatcher/appDispatcher') +const appStore = require('../../js/stores/appStore') +const windowConstants = require('../../js/constants/windowConstants') const Menu = electron.Menu -const MenuItem = electron.MenuItem +const CommonMenu = require('../common/commonMenu') const messages = require('../../js/constants/messages') const settings = require('../../js/constants/settings') +const siteTags = require('../../js/constants/siteTags') const dialog = electron.dialog -const appActions = require('../../js/actions/appActions') const { fileUrl } = require('../../js/lib/appUrlUtil') const menuUtil = require('./lib/menuUtil') const getSetting = require('../../js/settings').getSetting @@ -21,15 +26,13 @@ const {isSiteBookmarked} = require('../../js/state/siteUtil') const isDarwin = process.platform === 'darwin' const aboutUrl = 'https://brave.com/' -// Start off with an empty menu -let appMenu = Menu.buildFromTemplate([]) -Menu.setApplicationMenu(appMenu) - -// Value for history menu's "Bookmark Page" menu item; see createBookmarksSubmenu() -let isBookmarkChecked = false +let appMenu = null +// TODO(bridiver) - these should be handled in the appStore +let closedFrames = {} +let currentLocation = null // Submenu initialization -const createFileSubmenu = (CommonMenu) => { +const createFileSubmenu = () => { const submenu = [ CommonMenu.newTabMenuItem(), CommonMenu.newPrivateTabMenuItem(), @@ -121,7 +124,7 @@ const createFileSubmenu = (CommonMenu) => { return submenu } -const createEditSubmenu = (CommonMenu) => { +const createEditSubmenu = () => { const submenu = [ { label: locale.translation('undo'), @@ -186,7 +189,7 @@ const createEditSubmenu = (CommonMenu) => { return submenu } -const createViewSubmenu = (CommonMenu) => { +const createViewSubmenu = () => { return [ { label: locale.translation('actualSize'), @@ -287,7 +290,7 @@ const createViewSubmenu = (CommonMenu) => { ] } -const createHistorySubmenu = (CommonMenu) => { +const createHistorySubmenu = () => { let submenu = [ { label: locale.translation('home'), @@ -347,8 +350,7 @@ const createHistorySubmenu = (CommonMenu) => { } } ] - - submenu = submenu.concat(menuUtil.createRecentlyClosedMenuItems()) + submenu = submenu.concat(menuUtil.createRecentlyClosedMenuItems(Immutable.fromJS(Object.keys(closedFrames).map(key => closedFrames[key])))) submenu.push( // TODO: recently visited @@ -363,13 +365,17 @@ const createHistorySubmenu = (CommonMenu) => { return submenu } -const createBookmarksSubmenu = (CommonMenu) => { +const isCurrentLocationBookmarked = () => { + return isSiteBookmarked(appStore.getState().get('sites'), Immutable.fromJS({location: currentLocation})) +} + +const createBookmarksSubmenu = () => { let submenu = [ { label: locale.translation('bookmarkPage'), type: 'checkbox', accelerator: 'CmdOrCtrl+D', - checked: isBookmarkChecked, // NOTE: checked status is updated via updateBookmarkedStatus() + checked: isCurrentLocationBookmarked(), click: function (item, focusedWindow) { var msg = item.checked ? messages.SHORTCUT_ACTIVE_FRAME_REMOVE_BOOKMARK @@ -389,7 +395,7 @@ const createBookmarksSubmenu = (CommonMenu) => { CommonMenu.importBookmarksMenuItem() ] - const bookmarks = menuUtil.createBookmarkMenuItems() + const bookmarks = menuUtil.createBookmarkMenuItems(appStore.getState().get('sites')) if (bookmarks.length > 0) { submenu.push(CommonMenu.separatorMenuItem) submenu = submenu.concat(bookmarks) @@ -398,7 +404,7 @@ const createBookmarksSubmenu = (CommonMenu) => { return submenu } -const createWindowSubmenu = (CommonMenu) => { +const createWindowSubmenu = () => { return [ { label: locale.translation('minimize'), @@ -441,7 +447,7 @@ const createWindowSubmenu = (CommonMenu) => { ] } -const createHelpSubmenu = (CommonMenu) => { +const createHelpSubmenu = () => { const submenu = [ CommonMenu.reportAnIssueMenuItem(), CommonMenu.separatorMenuItem, @@ -465,7 +471,7 @@ const createHelpSubmenu = (CommonMenu) => { return submenu } -const createDebugSubmenu = (CommonMenu) => { +const createDebugSubmenu = () => { return [ { // Makes future renderer processes pause when they are created until a debugger appears. @@ -510,13 +516,13 @@ const createDebugSubmenu = (CommonMenu) => { * Will only build the initial menu, which is mostly static items * Dynamic items (Bookmarks, History) get updated w/ updateMenu */ -const createMenu = (CommonMenu) => { +const createMenu = () => { const template = [ - { label: locale.translation('file'), submenu: createFileSubmenu(CommonMenu) }, - { label: locale.translation('edit'), submenu: createEditSubmenu(CommonMenu) }, - { label: locale.translation('view'), submenu: createViewSubmenu(CommonMenu) }, - { label: locale.translation('history'), submenu: createHistorySubmenu(CommonMenu) }, - { label: locale.translation('bookmarks'), submenu: createBookmarksSubmenu(CommonMenu) }, + { label: locale.translation('file'), submenu: createFileSubmenu() }, + { label: locale.translation('edit'), submenu: createEditSubmenu() }, + { label: locale.translation('view'), submenu: createViewSubmenu() }, + { label: locale.translation('history'), submenu: createHistorySubmenu() }, + { label: locale.translation('bookmarks'), submenu: createBookmarksSubmenu() }, { label: locale.translation('bravery'), submenu: [ @@ -524,12 +530,12 @@ const createMenu = (CommonMenu) => { CommonMenu.braverySiteMenuItem() ] }, - { label: locale.translation('window'), submenu: createWindowSubmenu(CommonMenu), role: 'window' }, - { label: locale.translation('help'), submenu: createHelpSubmenu(CommonMenu), role: 'help' } + { label: locale.translation('window'), submenu: createWindowSubmenu(), role: 'window' }, + { label: locale.translation('help'), submenu: createHelpSubmenu(), role: 'help' } ] if (process.env.NODE_ENV === 'development') { - template.push({ label: 'Debug', submenu: createDebugSubmenu(CommonMenu) }) + template.push({ label: 'Debug', submenu: createDebugSubmenu() }) } if (isDarwin) { @@ -572,7 +578,7 @@ const createMenu = (CommonMenu) => { }) } - const oldMenu = appMenu + let oldMenu = appMenu appMenu = Menu.buildFromTemplate(template) Menu.setApplicationMenu(appMenu) if (oldMenu) { @@ -580,73 +586,64 @@ const createMenu = (CommonMenu) => { } } -const updateMenu = (CommonMenu, appState, windowData) => { - const updated = menuUtil.checkForUpdate(appState, windowData) - if (updated.nothingUpdated) { - return - } - - // When bookmarks are removed via AppStore (context menu, etc), `isBookmarkChecked` needs to be recalculated - if (windowData && windowData.get('location')) { - isBookmarkChecked = isSiteBookmarked(appState.get('sites'), Immutable.fromJS({location: windowData.get('location')})) - } - - // Only rebuild menus when necessary - - if (updated.settings || updated.closedFrames) { - let historyMenu = menuUtil.getParentMenuDetails(appMenu, locale.translation('history')) - if (historyMenu && historyMenu.menu && historyMenu.menu.submenu && historyMenu.index !== -1) { - const menu = historyMenu.menu.submenu - const menuItems = createHistorySubmenu(CommonMenu) - menu.clear() - menuItems.forEach((item) => menu.append(new MenuItem(item))) - } - } - - if (updated.sites) { - let bookmarksMenu = menuUtil.getParentMenuDetails(appMenu, locale.translation('bookmarks')) - if (bookmarksMenu && bookmarksMenu.menu && bookmarksMenu.menu.submenu && bookmarksMenu.index !== -1) { - const menu = bookmarksMenu.menu.submenu - const menuItems = createBookmarksSubmenu(CommonMenu) - menu.clear() - menuItems.forEach((item) => menu.append(new MenuItem(item))) - } +const doAction = (action) => { + switch (action.actionType) { + case windowConstants.WINDOW_SET_FOCUSED_FRAME: + // TODO check/uncheck menu item instead of recreating menu + currentLocation = action.frameProps.get('location') + let menuItem = menuUtil.getMenuItem(appMenu, locale.translation('bookmarkPage')) + menuItem.checked = isCurrentLocationBookmarked() + break + case windowConstants.WINDOW_UNDO_CLOSED_FRAME: + appDispatcher.waitFor([appStore.dispatchToken], () => { + delete closedFrames[action.frameProps.get('location')] + createMenu() + }) + break + case windowConstants.WINDOW_CLEAR_CLOSED_FRAMES: + appDispatcher.waitFor([appStore.dispatchToken], () => { + closedFrames = {} + createMenu() + }) + break + case windowConstants.WINDOW_CLOSE_FRAME: + appDispatcher.waitFor([appStore.dispatchToken], () => { + if (!action.frameProps.get('isPrivate') && action.frameProps.get('location') !== 'about:newtab') { + closedFrames[action.frameProps.get('location')] = action.frameProps + createMenu() + } + }) + break + case appConstants.APP_ADD_SITE: + if (action.tag === siteTags.BOOKMARK) { + appDispatcher.waitFor([appStore.dispatchToken], () => { + createMenu() + }) + } + break + case appConstants.APP_REMOVE_SITE: + if (action.tag === siteTags.BOOKMARK) { + appDispatcher.waitFor([appStore.dispatchToken], () => { + createMenu() + }) + } + break + case appConstants.APP_CLEAR_HISTORY: + if (action.tag === siteTags.BOOKMARK) { + appDispatcher.waitFor([appStore.dispatchToken], () => { + createMenu() + }) + } + break + default: } - - Menu.setApplicationMenu(appMenu) } /** * Sets up the menu. * @param {Object} appState - Application state. Used to fetch bookmarks and settings (like homepage) - * @param {Object} windowData - Information specific to the current window (recently closed tabs, etc) */ -module.exports.rebuild = (appState, windowData) => { - // The menu will always be called once localization is done - // so don't bother loading anything until it is done. - if (!locale.initialized) { - return - } - - // This needs to be within the init method to handle translations - const CommonMenu = require('../common/commonMenu') - if (appMenu.items.length === 0) { - createMenu(CommonMenu) - } else { - updateMenu(CommonMenu, appState, windowData) - } -} - -/** - * Called from navigationBar.js; used to update bookmarks menu status - * @param {boolean} isBookmarked - true if the currently viewed site is bookmarked - */ -module.exports.updateBookmarkedStatus = (isBookmarked) => { - const menuItem = menuUtil.getMenuItem(locale.translation('bookmarkPage')) - if (menuItem) { - menuItem.checked = isBookmarked - } - // menu may be rebuilt without the location changing - // this holds the last known status - isBookmarkChecked = isBookmarked +module.exports.init = (appState) => { + createMenu() + appDispatcher.register(doAction) } diff --git a/app/index.js b/app/index.js index bbc67cbbdc2..b6b05481183 100644 --- a/app/index.js +++ b/app/index.js @@ -24,8 +24,6 @@ if (process.platform === 'win32') { require('./windowsInit') } -var locale = require('./locale') - const path = require('path') const electron = require('electron') const app = electron.app @@ -64,9 +62,9 @@ const showAbout = require('./aboutDialog').showAbout const urlParse = require('url').parse const CryptoUtil = require('../js/lib/cryptoUtil') const keytar = require('keytar') -const settings = require('../js/constants/settings') const siteSettings = require('../js/state/siteSettings') const spellCheck = require('./spellCheck') +const locale = require('./locale') const ledger = require('./ledger') const flash = require('../js/flash') const contentSettings = require('../js/state/contentSettings') @@ -227,9 +225,7 @@ const initiateSessionStateSave = (beforeQuit) => { } } -let loadAppStatePromise = SessionStore.loadAppState().catch(() => { - return SessionStore.defaultAppState() -}) +let loadAppStatePromise = SessionStore.loadAppState() let flashInitialized = false @@ -249,7 +245,6 @@ loadAppStatePromise.then((initialState) => { return } } - app.setLocale(initialState.settings[settings.LANGUAGE]) }) app.on('ready', () => { @@ -322,13 +317,6 @@ app.on('ready', () => { saveIfAllCollected() }) - // Window state is fetched via the renderer process; this is fired once it's retrieved - ipcMain.on(messages.RESPONSE_MENU_DATA_FOR_WINDOW, (wnd, windowData) => { - if (windowData) { - Menu.rebuild(AppStore.getState(), Immutable.fromJS(windowData)) - } - }) - ipcMain.on(messages.LAST_WINDOW_STATE, (wnd, data) => { if (data) { lastWindowState = data @@ -383,19 +371,13 @@ app.on('ready', () => { }) loadAppStatePromise.then((initialState) => { - // Initiate the translation for a configured language and - // reset the browser window. This will default to en-US if - // not yet configured. - locale.init(initialState.settings[settings.LANGUAGE], (strings) => { - Menu.rebuild(AppStore.getState(), null) - }) - // Do this after loading the state // For tests we always want to load default app state const loadedPerWindowState = initialState.perWindowState delete initialState.perWindowState initialState.flashInitialized = flashInitialized appActions.setState(Immutable.fromJS(initialState)) + Menu.init(initialState, null) return loadedPerWindowState }).then((loadedPerWindowState) => { basicAuth.init() @@ -461,12 +443,6 @@ app.on('ready', () => { } }) - ipcMain.on(messages.UPDATE_MENU_BOOKMARKED_STATUS, (e, isBookmarked) => { - if (typeof isBookmarked === 'boolean') { - Menu.updateBookmarkedStatus(isBookmarked) - } - }) - ipcMain.on(messages.SET_CLIPBOARD, (e, text) => { electron.clipboard.writeText(text) }) @@ -525,14 +501,6 @@ app.on('ready', () => { // save app state every 5 minutes regardless of update frequency sessionStateSaveInterval = setInterval(initiateSessionStateSave, 1000 * 60 * 5) - AppStore.addChangeListener(() => { - if (BrowserWindow.getFocusedWindow()) { - BrowserWindow.getFocusedWindow().webContents.send(messages.REQUEST_MENU_DATA_FOR_WINDOW) - } else { - Menu.rebuild(AppStore.getState(), null) - } - }) - ledger.init() ipcMain.on(messages.LEDGER_CREATE_WALLET, () => { diff --git a/app/locale.js b/app/locale.js index 64f24becc96..55cfe5f3cc7 100644 --- a/app/locale.js +++ b/app/locale.js @@ -270,9 +270,24 @@ const defaultLocale = function () { // Initialize translations for a language providing an optional // callback executed after the translation caching process // is complete. -exports.init = function (language, cb) { - // Default to noop callback - cb = cb || function () {} +exports.init = function (language) { + // If this is in the main process + if (ipcMain) { + // Respond to requests for translations from the renderer process + ipcMain.on('translations', function (event, arg) { + // Return the entire set of translations synchronously + event.returnValue = translations + }) + + // TODO: There shouldn't need to be a REQUEST_LANGUAGE event at all + // Respond to requests for the currently configured language code + ipcMain.on(REQUEST_LANGUAGE, function (event) { + event.sender.send(LANGUAGE, { + langCode: lang, + languageCodes: availableLanguages + }) + }) + } // Currently selected language identifier I.e. 'en-US' lang = language || defaultLocale() @@ -303,31 +318,11 @@ exports.init = function (language, cb) { // Translate the renderer identifiers var identifiers = rendererIdentifiers() - ctx.formatValues.apply(ctx, identifiers).then(function (values) { + return ctx.formatValues.apply(ctx, identifiers).then(function (values) { // Cache the translations for later retrieval values.forEach(function (value, idx) { translations[identifiers[idx]] = value }) - // Signal when complete - exports.initialized = true - cb(translations) - }) -} - -// If this is in the main process -if (ipcMain) { - // Respond to requests for translations from the renderer process - ipcMain.on('translations', function (event, arg) { - // Return the entire set of translations synchronously - event.returnValue = translations - }) - - // TODO: There shouldn't need to be a REQUEST_LANGUAGE event at all - // Respond to requests for the currently configured language code - ipcMain.on(REQUEST_LANGUAGE, function (event) { - event.sender.send(LANGUAGE, { - langCode: lang, - languageCodes: availableLanguages - }) + return lang }) } diff --git a/app/sessionStore.js b/app/sessionStore.js index d6c587af51e..25109121e37 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -16,6 +16,7 @@ const fs = require('fs') const path = require('path') const electron = require('electron') const app = electron.app +const locale = require('./locale') const UpdateStatus = require('../js/constants/updateStatus') const settings = require('../js/constants/settings') const downloadStates = require('../js/constants/downloadStates') @@ -308,13 +309,7 @@ module.exports.loadAppState = () => { let data try { data = fs.readFileSync(getStoragePath()) - } catch (e) { - } - - if (!data) { - reject() - return - } + } catch (e) {} try { data = JSON.parse(data) @@ -337,7 +332,6 @@ module.exports.loadAppState = () => { data.autofill.creditCards = creditCards } } - // xml migration if (data.settings) { if (data.settings[settings.DEFAULT_SEARCH_ENGINE] === 'content/search/google.xml') { @@ -347,35 +341,37 @@ module.exports.loadAppState = () => { data.settings[settings.DEFAULT_SEARCH_ENGINE] = 'DuckDuckGo' } } + // Clean app data here if it wasn't cleared on shutdown + if (data.cleanedOnShutdown !== true || data.lastAppVersion !== app.getVersion()) { + module.exports.cleanAppData(data, false) + } + data = Object.assign(module.exports.defaultAppState(), data) + data.cleanedOnShutdown = false + // Always recalculate the update status + if (data.updates) { + const updateStatus = data.updates.status + delete data.updates.status + // The process always restarts after an update so if the state + // indicates that a restart isn't wanted, close right away. + if (updateStatus === UpdateStatus.UPDATE_APPLYING_NO_RESTART) { + module.exports.saveAppState(data, true).then(() => { + // Exit immediately without doing the session store saving stuff + // since we want the same state saved except for the update status + app.exit(0) + }) + return + } + } } catch (e) { // TODO: Session state is corrupted, maybe we should backup this // corrupted value for people to report into support. console.log('could not parse data: ', data) - reject(e) - return - } - // Clean app data here if it wasn't cleared on shutdown - if (data.cleanedOnShutdown !== true || data.lastAppVersion !== app.getVersion()) { - module.exports.cleanAppData(data, false) - } - data = Object.assign(module.exports.defaultAppState(), data) - data.cleanedOnShutdown = false - // Always recalculate the update status - if (data.updates) { - const updateStatus = data.updates.status - delete data.updates.status - // The process always restarts after an update so if the state - // indicates that a restart isn't wanted, close right away. - if (updateStatus === UpdateStatus.UPDATE_APPLYING_NO_RESTART) { - module.exports.saveAppState(data, true).then(() => { - // Exit immediately without doing the session store saving stuff - // since we want the same state saved except for the update status - app.exit(0) - }) - return - } + data = exports.defaultAppState() } - resolve(data) + locale.init(data.settings[settings.LANGUAGE]).then((locale) => { + app.setLocale(locale) + resolve(data) + }) }) } diff --git a/js/components/addEditBookmark.js b/js/components/addEditBookmark.js index dc903c25034..3705e6f02d0 100644 --- a/js/components/addEditBookmark.js +++ b/js/components/addEditBookmark.js @@ -10,11 +10,9 @@ const windowActions = require('../actions/windowActions') const appActions = require('../actions/appActions') const KeyCodes = require('../constants/keyCodes') const siteTags = require('../constants/siteTags') -const messages = require('../constants/messages') const settings = require('../constants/settings') const siteUtil = require('../state/siteUtil') const getSetting = require('../settings').getSetting -const ipc = global.require('electron').ipcRenderer class AddEditBookmark extends ImmutableComponent { constructor () { @@ -97,9 +95,6 @@ class AddEditBookmark extends ImmutableComponent { ) appActions.changeSetting(settings.SHOW_BOOKMARKS_TOOLBAR, !isFirstBookmark || showBookmarksToolbar) } - updateMenuBookmarkedStatus (isBookmarked) { - ipc.send(messages.UPDATE_MENU_BOOKMARKED_STATUS, isBookmarked) - } onSave () { // First check if the title of the currentDetail is set if (!this.bookmarkNameValid) { @@ -109,13 +104,11 @@ class AddEditBookmark extends ImmutableComponent { this.showToolbarOnFirstBookmark() const tag = this.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK appActions.addSite(this.props.currentDetail, tag, this.props.originalDetail, this.props.destinationDetail) - this.updateMenuBookmarkedStatus(true) this.onClose() } onRemoveBookmark () { const tag = this.isFolder ? siteTags.BOOKMARK_FOLDER : siteTags.BOOKMARK appActions.removeSite(this.props.currentDetail, tag) - this.updateMenuBookmarkedStatus(false) this.onClose() } get displayBookmarkName () { diff --git a/js/components/navigationBar.js b/js/components/navigationBar.js index d7334793548..2858aec5066 100644 --- a/js/components/navigationBar.js +++ b/js/components/navigationBar.js @@ -84,8 +84,6 @@ class NavigationBar extends ImmutableComponent { componentDidMount () { ipc.on(messages.SHORTCUT_ACTIVE_FRAME_BOOKMARK, () => this.onToggleBookmark()) ipc.on(messages.SHORTCUT_ACTIVE_FRAME_REMOVE_BOOKMARK, () => this.onToggleBookmark()) - // Set initial checked/unchecked status in Bookmarks menu - ipc.send(messages.UPDATE_MENU_BOOKMARKED_STATUS, this.bookmarked) } get showNoScriptInfo () { @@ -97,17 +95,6 @@ class NavigationBar extends ImmutableComponent { } componentDidUpdate (prevProps) { - // Update the app menu to reflect whether the current page is bookmarked - const prevBookmarked = this.props.activeFrameKey !== undefined && - siteUtil.isSiteBookmarked(prevProps.sites, Immutable.fromJS({ - location: prevProps.location, - partitionNumber: prevProps.partitionNumber, - title: prevProps.title - })) - - if (this.bookmarked !== prevBookmarked) { - ipc.send(messages.UPDATE_MENU_BOOKMARKED_STATUS, this.bookmarked) - } if (this.props.noScriptIsVisible && !this.showNoScriptInfo) { // There are no blocked scripts, so hide the noscript dialog. windowActions.setNoScriptVisible(false) diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index b80f69db827..baf49dd0370 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -44,7 +44,9 @@ const AppConstants = { APP_SET_LOGIN_RESPONSE_DETAIL: _, APP_WINDOW_BLURRED: _, APP_IDLE_STATE_CHANGED: _, - APP_CHANGE_NEW_TAB_DETAIL: _ + APP_CHANGE_NEW_TAB_DETAIL: _, + APP_TAB_CREATED: _, + APP_TAB_DESTROYED: _ } module.exports = mapValuesByKeys(AppConstants) diff --git a/js/constants/messages.js b/js/constants/messages.js index 46ae4c79eb4..fe733b13efd 100644 --- a/js/constants/messages.js +++ b/js/constants/messages.js @@ -104,10 +104,6 @@ const messages = { LAST_WINDOW_STATE: _, UNDO_CLOSED_WINDOW: _, CLEAR_CLOSED_FRAMES: _, - // Menu rebuilding - REQUEST_MENU_DATA_FOR_WINDOW: _, - RESPONSE_MENU_DATA_FOR_WINDOW: _, - UPDATE_MENU_BOOKMARKED_STATUS: _, /** @isBookmarked {boolean} should menu show "Bookmark Page" as checked */ // Ad block, safebrowsing, and tracking protection BLOCKED_RESOURCE: _, BLOCKED_PAGE: _, diff --git a/js/dispatcher/appDispatcher.js b/js/dispatcher/appDispatcher.js index 1da0b6d8fcd..c11dfc63692 100644 --- a/js/dispatcher/appDispatcher.js +++ b/js/dispatcher/appDispatcher.js @@ -93,7 +93,11 @@ if (process.type === 'browser') { let registrant = event.sender const callback = function (payload) { try { - registrant.send(messages.DISPATCH_ACTION, Serializer.serialize(payload)) + if (registrant.isDestroyed()) { + appDispatcher.unregister(callback) + } else { + registrant.send(messages.DISPATCH_ACTION, Serializer.serialize(payload)) + } } catch (e) { console.error('unregistering callback', e) appDispatcher.unregister(callback) diff --git a/js/entry.js b/js/entry.js index e972296d8b4..b95e2cc0c7f 100644 --- a/js/entry.js +++ b/js/entry.js @@ -35,7 +35,6 @@ const messages = require('./constants/messages') const Immutable = require('immutable') const patch = require('immutablepatch') const l10n = require('./l10n') -const FrameStateUtil = require('./state/frameStateUtil') // don't allow scaling or zooming of the ui webFrame.setPageScaleLimits(1, 1) @@ -55,16 +54,6 @@ ipc.on(messages.REQUEST_WINDOW_STATE, () => { ipc.send(messages.RESPONSE_WINDOW_STATE, windowStore.getState().toJS()) }) -ipc.on(messages.REQUEST_MENU_DATA_FOR_WINDOW, () => { - const windowState = windowStore.getState() - const activeFrame = FrameStateUtil.getActiveFrame(Immutable.fromJS(windowState)) - const windowData = { - location: activeFrame ? activeFrame.get('location') : null, - closedFrames: windowState.get('closedFrames').toJS() - } - ipc.send(messages.RESPONSE_MENU_DATA_FOR_WINDOW, windowData) -}) - if (process.env.NODE_ENV === 'test') { window.appStoreRenderer = appStoreRenderer window.windowActions = windowActions diff --git a/js/stores/appStore.js b/js/stores/appStore.js index f1ccd7f71d1..e8094a3b305 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -155,10 +155,6 @@ const createWindow = (browserOpts, defaults, frameOpts, windowState) => { appActions.windowBlurred(mainWindow.id) }) - mainWindow.on('focus', function () { - mainWindow.webContents.send(messages.REQUEST_MENU_DATA_FOR_WINDOW) - }) - mainWindow.on('resize', function (evt) { // the default window size is whatever the last window resize was appActions.setDefaultWindowSize(evt.sender.getSize()) @@ -540,7 +536,6 @@ const handleAppAction = (action) => { if (action.clearDataDetail.get('browserHistory')) { handleAppAction({actionType: AppConstants.APP_CLEAR_HISTORY}) BrowserWindow.getAllWindows().forEach((wnd) => wnd.webContents.send(messages.CLEAR_CLOSED_FRAMES)) - BrowserWindow.getAllWindows().forEach((wnd) => wnd.webContents.send(messages.REQUEST_MENU_DATA_FOR_WINDOW)) } if (action.clearDataDetail.get('downloadHistory')) { handleAppAction({actionType: AppConstants.APP_CLEAR_COMPLETED_DOWNLOADS}) diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 302e0c4cedb..3ee517d6aa9 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -442,13 +442,6 @@ const doAction = (action) => { windowState = windowState.merge(FrameStateUtil.removeFrame(windowState.get('frames'), windowState.get('tabs'), windowState.get('closedFrames'), frameProps.set('closedAtIndex', index), activeFrameKey)) - // History menu needs update (since it shows "Recently Closed" items) - const activeFrameLocation = FrameStateUtil.getActiveFrame(windowState).get('location') - const windowData = { - location: activeFrameLocation, - closedFrames: windowState.get('closedFrames').toJS() - } - ipc.send(messages.RESPONSE_MENU_DATA_FOR_WINDOW, windowData) // If we reach the limit of opened tabs per page while closing tabs, switch to // the active tab's page otherwise the user will hang on empty page let totalOpenTabs = windowState.get('frames').filter((frame) => !frame.get('pinnedLocation')).size diff --git a/test/lib/brave.js b/test/lib/brave.js index 8502703a517..00c2d1288c2 100644 --- a/test/lib/brave.js +++ b/test/lib/brave.js @@ -35,7 +35,12 @@ const rmDir = (dirPath) => { } } } - fs.rmdirSync(dirPath) + try { + fs.rmdirSync(dirPath) + } catch (e) { + console.error(e) + return + } } var promiseMapSeries = function (array, iterator) { diff --git a/test/unit/lib/menuUtilTest.js b/test/unit/lib/menuUtilTest.js index 4552c50e814..e5ad3b06934 100644 --- a/test/unit/lib/menuUtilTest.js +++ b/test/unit/lib/menuUtilTest.js @@ -93,72 +93,24 @@ describe('menuUtil', function () { }) }) - describe('checkForUpdate', function () { - const appStateSettings = Immutable.fromJS({ - settings: { - key: 'value' - } - }) - const appStateSites = Immutable.fromJS({ - sites: { - key: 'value' - } - }) - const windowStateClosedFrames = Immutable.fromJS({ - closedFrames: [{ - frameId: 1 - }] - }) - it('sets updated.nothing to false if `settings` was updated', function () { - const updated = menuUtil.checkForUpdate(appStateSettings, null) - assert.equal(updated.nothing, false) - }) - it('sets updated.nothing to false if `sites` was updated', function () { - const updated = menuUtil.checkForUpdate(appStateSites, null) - assert.equal(updated.nothing, false) - console.log(menuUtil.lastSites) - }) - it('sets updated.nothing to false if `closedFrames` was updated', function () { - const updated = menuUtil.checkForUpdate(null, windowStateClosedFrames) - assert.equal(updated.nothing, false) - }) - it('leaves updated.nothing as true if nothing was updated', function () { - const updated = menuUtil.checkForUpdate(null, null) - assert.equal(updated.nothing, true) - }) - it('does not save falsey values', function () { - menuUtil.checkForUpdate(appStateSites, null) - menuUtil.checkForUpdate(null, null) - // NOTE: there should be no change, since last calls values weren't recorded - const updated = menuUtil.checkForUpdate(appStateSites, null) - assert.equal(updated.nothing, true) - }) - }) - describe('createBookmarkMenuItems', function () { it('returns an array of items w/ the bookmark tag', function () { - const appStateSites = Immutable.fromJS({ - sites: [ - { tags: [siteTags.BOOKMARK], title: 'my website', location: 'https://brave.com' } - ] - }) + const appStateSites = Immutable.fromJS([ + { tags: [siteTags.BOOKMARK], title: 'my website', location: 'https://brave.com' } + ]) - menuUtil.checkForUpdate(appStateSites, null) - const menuItems = menuUtil.createBookmarkMenuItems() + const menuItems = menuUtil.createBookmarkMenuItems(appStateSites) assert.equal(Array.isArray(menuItems), true) assert.equal(menuItems.length, 1) assert.equal(menuItems[0].label, 'my website') }) it('prefers the customTitle field for the bookmark title (over the page title)', function () { - const appStateSites = Immutable.fromJS({ - sites: [ - { tags: [siteTags.BOOKMARK], customTitle: 'use this', title: 'not this', location: 'https://brave.com' } - ] - }) + const appStateSites = Immutable.fromJS([ + { tags: [siteTags.BOOKMARK], customTitle: 'use this', title: 'not this', location: 'https://brave.com' } + ]) - menuUtil.checkForUpdate(appStateSites, null) - const menuItems = menuUtil.createBookmarkMenuItems() + const menuItems = menuUtil.createBookmarkMenuItems(appStateSites) assert.equal(menuItems[0].label, 'use this') }) @@ -169,8 +121,7 @@ describe('menuUtil', function () { ] }) - menuUtil.checkForUpdate(appStateSites, null) - const menuItems = menuUtil.createBookmarkMenuItems() + const menuItems = menuUtil.createBookmarkMenuItems(appStateSites) assert.deepEqual(menuItems, []) }) @@ -181,34 +132,26 @@ describe('menuUtil', function () { ] }) - menuUtil.checkForUpdate(appStateSites, null) - const menuItems = menuUtil.createBookmarkMenuItems() + const menuItems = menuUtil.createBookmarkMenuItems(appStateSites) assert.deepEqual(menuItems, []) }) it('does not count pinned tabs as bookmarks', function () { - const appStateSites = Immutable.fromJS({ - sites: [ - { tags: [siteTags.PINNED], title: 'pinned site', location: 'https://pinned-website.com' }, - { tags: [siteTags.BOOKMARK], title: 'my website', location: 'https://brave.com' } - ] - }) - menuUtil.checkForUpdate(appStateSites, null) - const menuItems = menuUtil.createBookmarkMenuItems() + const appStateSites = Immutable.fromJS([ + { tags: [siteTags.PINNED], title: 'pinned site', location: 'https://pinned-website.com' }, + { tags: [siteTags.BOOKMARK], title: 'my website', location: 'https://brave.com' } + ]) + const menuItems = menuUtil.createBookmarkMenuItems(appStateSites) assert.equal(menuItems.length, 1) assert.equal(menuItems[0].label, 'my website') }) it('processes folders', function () { - const appStateSites = Immutable.fromJS({ - sites: [ - { tags: [siteTags.BOOKMARK_FOLDER], title: 'my folder', folderId: 123 }, - { tags: [siteTags.BOOKMARK], title: 'my website', location: 'https://brave.com', parentFolderId: 123 } - ] - }) - - menuUtil.checkForUpdate(appStateSites, null) - const menuItems = menuUtil.createBookmarkMenuItems() + const appStateSites = Immutable.fromJS([ + { tags: [siteTags.BOOKMARK_FOLDER], title: 'my folder', folderId: 123 }, + { tags: [siteTags.BOOKMARK], title: 'my website', location: 'https://brave.com', parentFolderId: 123 } + ]) + const menuItems = menuUtil.createBookmarkMenuItems(appStateSites) assert.equal(menuItems.length, 1) assert.equal(menuItems[0].label, 'my folder') @@ -216,15 +159,12 @@ describe('menuUtil', function () { assert.equal(menuItems[0].submenu[0].label, 'my website') }) it('considers customTitle when processing folders', function () { - const appStateSites = Immutable.fromJS({ - sites: [ - { tags: [siteTags.BOOKMARK_FOLDER], customTitle: 'use this', title: 'not this', folderId: 123 }, - { tags: [siteTags.BOOKMARK], title: 'my website', location: 'https://brave.com', parentFolderId: 123 } - ] - }) + const appStateSites = Immutable.fromJS([ + { tags: [siteTags.BOOKMARK_FOLDER], customTitle: 'use this', title: 'not this', folderId: 123 }, + { tags: [siteTags.BOOKMARK], title: 'my website', location: 'https://brave.com', parentFolderId: 123 } + ]) - menuUtil.checkForUpdate(appStateSites, null) - const menuItems = menuUtil.createBookmarkMenuItems() + const menuItems = menuUtil.createBookmarkMenuItems(appStateSites) assert.equal(menuItems.length, 1) assert.equal(menuItems[0].label, 'use this') @@ -233,47 +173,39 @@ describe('menuUtil', function () { describe('createRecentlyClosedMenuItems', function () { it('returns an array of closedFrames preceded by a separator and "Recently Closed" items', function () { - const windowStateClosedFrames = Immutable.fromJS({ - closedFrames: [{ - title: 'sample', - location: 'https://brave.com' - }] - }) - - menuUtil.checkForUpdate(null, windowStateClosedFrames) - const menuItems = menuUtil.createRecentlyClosedMenuItems() + const windowStateClosedFrames = Immutable.fromJS([{ + title: 'sample', + location: 'https://brave.com' + }]) + const menuItems = menuUtil.createRecentlyClosedMenuItems(windowStateClosedFrames) assert.equal(Array.isArray(menuItems), true) assert.equal(menuItems.length, 3) assert.equal(menuItems[0].type, 'separator') assert.equal(menuItems[1].label, 'RECENTLYCLOSED') assert.equal(menuItems[1].enabled, false) - assert.equal(menuItems[2].label, windowStateClosedFrames.get('closedFrames').first().get('title')) + assert.equal(menuItems[2].label, windowStateClosedFrames.first().get('title')) assert.equal(typeof menuItems[2].click === 'function', true) }) it('only shows the last 10 items', function () { - const windowStateClosedFrames = Immutable.fromJS({ - closedFrames: [ - { title: 'site01', location: 'https://brave01.com' }, - { title: 'site02', location: 'https://brave02.com' }, - { title: 'site03', location: 'https://brave03.com' }, - { title: 'site04', location: 'https://brave04.com' }, - { title: 'site05', location: 'https://brave05.com' }, - { title: 'site06', location: 'https://brave06.com' }, - { title: 'site07', location: 'https://brave07.com' }, - { title: 'site08', location: 'https://brave08.com' }, - { title: 'site09', location: 'https://brave09.com' }, - { title: 'site10', location: 'https://brave10.com' }, - { title: 'site11', location: 'https://brave11.com' } - ] - }) - - menuUtil.checkForUpdate(null, windowStateClosedFrames) - const menuItems = menuUtil.createRecentlyClosedMenuItems() + const windowStateClosedFrames = Immutable.fromJS([ + { title: 'site01', location: 'https://brave01.com' }, + { title: 'site02', location: 'https://brave02.com' }, + { title: 'site03', location: 'https://brave03.com' }, + { title: 'site04', location: 'https://brave04.com' }, + { title: 'site05', location: 'https://brave05.com' }, + { title: 'site06', location: 'https://brave06.com' }, + { title: 'site07', location: 'https://brave07.com' }, + { title: 'site08', location: 'https://brave08.com' }, + { title: 'site09', location: 'https://brave09.com' }, + { title: 'site10', location: 'https://brave10.com' }, + { title: 'site11', location: 'https://brave11.com' } + ]) + const menuItems = menuUtil.createRecentlyClosedMenuItems(windowStateClosedFrames) assert.equal(menuItems.length, 12) - assert.equal(menuItems[2].label, windowStateClosedFrames.get('closedFrames').get(1).get('title')) - assert.equal(menuItems[11].label, windowStateClosedFrames.get('closedFrames').get(10).get('title')) + assert.equal(menuItems[2].label, windowStateClosedFrames.get(1).get('title')) + assert.equal(menuItems[11].label, windowStateClosedFrames.get(10).get('title')) }) }) })