From 28baeee581352507c8017a705f29050ae6b49f55 Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Fri, 30 Jun 2017 14:45:10 +0200 Subject: [PATCH] Removes non-primitive types from TabPage Resolves #9793 Auditors: @bsclifton @bridiver Test Plan: --- app/renderer/components/tabs/tabPage.js | 22 +++-- app/renderer/reducers/contextMenuReducer.js | 75 ++++++++++++++-- js/actions/windowActions.js | 7 ++ js/constants/windowConstants.js | 3 +- js/contextMenus.js | 36 -------- .../reducers/contextMenuReducerTest.js | 86 +++++++++++++++++++ 6 files changed, 178 insertions(+), 51 deletions(-) create mode 100644 test/unit/app/renderer/reducers/contextMenuReducerTest.js diff --git a/app/renderer/components/tabs/tabPage.js b/app/renderer/components/tabs/tabPage.js index 58e308a7e0b..1758bc05cd8 100644 --- a/app/renderer/components/tabs/tabPage.js +++ b/app/renderer/components/tabs/tabPage.js @@ -19,7 +19,6 @@ const settings = require('../../../../js/constants/settings') // Utils const {getSetting} = require('../../../../js/settings') const cx = require('../../../../js/lib/classSet') -const {onTabPageContextMenu} = require('../../../../js/contextMenus') const {getCurrentWindowId} = require('../../currentWindow') const dndData = require('../../../../js/dndData') const frameStateUtil = require('../../../../js/state/frameStateUtil') @@ -30,6 +29,8 @@ class TabPage extends React.Component { super(props) this.onMouseEnter = this.onMouseEnter.bind(this) this.onMouseLeave = this.onMouseLeave.bind(this) + this.onContextMenu = this.onContextMenu.bind(this) + this.onClick = this.onClick.bind(this) } onMouseLeave () { @@ -69,6 +70,15 @@ class TabPage extends React.Component { e.preventDefault() } + onContextMenu (e) { + e.stopPropagation() + windowActions.onTabPageMenu(this.props.index) + } + + onClick () { + windowActions.setTabPageIndex(this.props.index) + } + mergeProps (state, ownProps) { const currentWindow = state.get('currentWindow') const frames = frameStateUtil.getNonPinnedFrames(currentWindow) || Immutable.List() @@ -90,7 +100,6 @@ class TabPage extends React.Component { const props = {} // used in renderer props.index = ownProps.index - props.tabPageFrames = tabPageFrames // TODO (nejc) only primitives props.isAudioPlaybackActive = isAudioPlaybackActive props.previewTabPage = getSetting(settings.SHOW_TAB_PREVIEWS) props.active = currentWindow.getIn(['ui', 'tabs', 'tabPageIndex']) === props.index @@ -113,10 +122,11 @@ class TabPage extends React.Component { className={cx({ tabPage: true, audioPlaybackActive: this.props.isAudioPlaybackActive, - active: this.props.active})} - onContextMenu={onTabPageContextMenu.bind(this, this.props.tabPageFrames)} - onClick={windowActions.setTabPageIndex.bind(this, this.props.index) - } /> + active: this.props.active + })} + onContextMenu={this.onContextMenu} + onClick={this.onClick} + /> } } diff --git a/app/renderer/reducers/contextMenuReducer.js b/app/renderer/reducers/contextMenuReducer.js index 5e878a032fe..c5e199aa25b 100644 --- a/app/renderer/reducers/contextMenuReducer.js +++ b/app/renderer/reducers/contextMenuReducer.js @@ -3,23 +3,31 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const assert = require('assert') +const Immutable = require('immutable') +const electron = require('electron') +const remote = electron.remote +const Menu = remote.Menu // Constants const config = require('../../../js/constants/config.js') const windowConstants = require('../../../js/constants/windowConstants') +const settings = require('../../../js/constants/settings') // State -const contextMenuState = require('../../common/state/contextMenuState.js') +const contextMenuState = require('../../common/state/contextMenuState') // Actions -const appActions = require('../../../js/actions/appActions.js') -const windowAction = require('../../../js/actions/windowActions.js') +const appActions = require('../../../js/actions/appActions') +const windowActions = require('../../../js/actions/windowActions') // Utils -const eventUtil = require('../../../js/lib/eventUtil.js') -const CommonMenu = require('../../common/commonMenu.js') +const eventUtil = require('../../../js/lib/eventUtil') +const CommonMenu = require('../../common/commonMenu') const locale = require('../../../js/l10n.js') -const { makeImmutable, isMap } = require('../../common/state/immutableUtil') +const {makeImmutable, isMap} = require('../../common/state/immutableUtil') +const {getSetting} = require('../../../js/settings') +const frameStateUtil = require('../../../js/state/frameStateUtil') +const {getCurrentWindow} = require('../../renderer/currentWindow') const validateAction = function (action) { action = makeImmutable(action) @@ -33,6 +41,54 @@ const validateState = function (state) { return state } +function generateMuteFrameList (framePropsList, muted) { + return framePropsList.map((frameProp) => { + return { + frameKey: frameProp.get('key'), + tabId: frameProp.get('tabId'), + muted: muted && frameProp.get('audioPlaybackActive') && !frameProp.get('audioMuted') + } + }) +} + +const onTabPageMenu = function (state, action) { + action = validateAction(action) + state = validateState(state) + + const index = action.get('index') + if (index == null || index < 0) { + return + } + + const frames = frameStateUtil.getNonPinnedFrames(state) || Immutable.List() + const tabsPerPage = Number(getSetting(settings.TABS_PER_PAGE)) + const tabPageFrames = frames.slice(index * tabsPerPage, (index * tabsPerPage) + tabsPerPage) + + if (tabPageFrames.isEmpty()) { + return + } + + const template = [{ + label: locale.translation('unmuteTabs'), + click: () => { + windowActions.muteAllAudio(generateMuteFrameList(tabPageFrames, false)) + } + }, { + label: locale.translation('muteTabs'), + click: () => { + windowActions.muteAllAudio(generateMuteFrameList(tabPageFrames, true)) + } + }, { + label: locale.translation('closeTabPage'), + click: () => { + windowActions.closeFrames(tabPageFrames) + } + }] + + const tabPageMenu = Menu.buildFromTemplate(template) + tabPageMenu.popup(getCurrentWindow()) +} + const onLongBackHistory = (state, action) => { action = validateAction(action) state = validateState(state) @@ -72,7 +128,7 @@ const onLongBackHistory = (state, action) => { appActions.createTabRequested({ url: 'about:history' }) - windowAction.setContextMenuDetail() + windowActions.setContextMenuDetail() } }) @@ -125,7 +181,7 @@ const onLongForwardHistory = (state, action) => { appActions.createTabRequested({ url: 'about:history' }) - windowAction.setContextMenuDetail() + windowActions.setContextMenuDetail() } }) @@ -147,6 +203,9 @@ const contextMenuReducer = (windowState, action) => { case windowConstants.WINDOW_ON_GO_FORWARD_LONG: windowState = onLongForwardHistory(windowState, action) break + case windowConstants.WINDOW_ON_TAB_PAGE_MENU: + onTabPageMenu(windowState, action) + break } return windowState diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 1a50eeb4ee7..bf18be3f7fe 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -1142,6 +1142,13 @@ const windowActions = { url, error }) + }, + + onTabPageMenu: function (index) { + dispatch({ + actionType: windowConstants.WINDOW_ON_TAB_PAGE_MENU, + index + }) } } diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 6a653a956d2..cf0ccf5529e 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -101,7 +101,8 @@ const windowConstants = { WINDOW_ON_GO_BACK_LONG: _, WINDOW_ON_GO_FORWARD_LONG: _, WINDOW_CLOSE_OTHER_FRAMES: _, - WINDOW_ON_CERT_ERROR: _ + WINDOW_ON_CERT_ERROR: _, + WINDOW_ON_TAB_PAGE_MENU: _ } module.exports = mapValuesByKeys(windowConstants) diff --git a/js/contextMenus.js b/js/contextMenus.js index f7c78474555..6c282607932 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -77,35 +77,6 @@ const addFolderMenuItem = (closestDestinationDetail, isParent) => { } } -function tabPageTemplateInit (framePropsList) { - return [{ - label: locale.translation('unmuteTabs'), - click: () => { - windowActions.muteAllAudio(generateMuteFrameList(framePropsList, false)) - } - }, { - label: locale.translation('muteTabs'), - click: () => { - windowActions.muteAllAudio(generateMuteFrameList(framePropsList, true)) - } - }, { - label: locale.translation('closeTabPage'), - click: () => { - windowActions.closeFrames(framePropsList) - } - }] -} - -function generateMuteFrameList (framePropsList, muted) { - return framePropsList.map((frameProp) => { - return { - frameKey: frameProp.get('key'), - tabId: frameProp.get('tabId'), - muted: muted && frameProp.get('audioPlaybackActive') && !frameProp.get('audioMuted') - } - }) -} - function urlBarTemplateInit (searchDetail, activeFrame, e) { const items = getEditableItems(window.getSelection().toString()) const clipboardText = clipboard.readText() @@ -1378,12 +1349,6 @@ function onDownloadsToolbarContextMenu (downloadId, downloadItem, e) { downloadsToolbarMenu.popup(getCurrentWindow()) } -function onTabPageContextMenu (framePropsList, e) { - e.stopPropagation() - const tabPageMenu = Menu.buildFromTemplate(tabPageTemplateInit(framePropsList)) - tabPageMenu.popup(getCurrentWindow()) -} - function onUrlBarContextMenu (e) { e.stopPropagation() const searchDetail = appStoreRenderer.state.get('searchDetail') @@ -1478,7 +1443,6 @@ module.exports = { onNewTabContextMenu, onTabsToolbarContextMenu, onDownloadsToolbarContextMenu, - onTabPageContextMenu, onUrlBarContextMenu, onFindBarContextMenu, onSiteDetailContextMenu, diff --git a/test/unit/app/renderer/reducers/contextMenuReducerTest.js b/test/unit/app/renderer/reducers/contextMenuReducerTest.js new file mode 100644 index 00000000000..3e561ad9562 --- /dev/null +++ b/test/unit/app/renderer/reducers/contextMenuReducerTest.js @@ -0,0 +1,86 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +/* global describe, it, before, beforeEach, after, afterEach */ +const mockery = require('mockery') +const assert = require('assert') +const sinon = require('sinon') +const Immutable = require('immutable') +const fakeElectron = require('../../../lib/fakeElectron') +const windowConstants = require('../../../../../js/constants/windowConstants') + +describe('contextMenuReducer', function () { + let fakeElectronMenu, contextMenuReducer + + const fakeLocale = { + translation: (token) => { return token } + } + + before(function () { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + mockery.registerMock('electron', fakeElectron) + mockery.registerMock('../../../js/l10n.js', fakeLocale) + fakeElectronMenu = require('../../../lib/fakeElectronMenu') + contextMenuReducer = require('../../../../../app/renderer/reducers/contextMenuReducer') + }) + + after(function () { + mockery.disable() + }) + + describe('WINDOW_ON_TAB_PAGE_MENU', function () { + let menuBuildFromTemplateSpy, menuPopupSpy, state + + before(function () { + state = Immutable.fromJS({ + frames: [{ + tabId: 1, + key: 1 + }], + framesInternal: { + index: { + 1: 0 + }, + tabIndex: { + 1: 0 + } + } + }) + }) + + beforeEach(function () { + menuBuildFromTemplateSpy = sinon.spy(fakeElectron.remote.Menu, 'buildFromTemplate') + menuPopupSpy = sinon.spy(fakeElectronMenu, 'popup') + }) + + afterEach(function () { + menuBuildFromTemplateSpy.restore() + menuPopupSpy.restore() + }) + + it('index is outside the scope', function () { + const action = { + actionType: windowConstants.WINDOW_ON_TAB_PAGE_MENU, + index: -1 + } + contextMenuReducer(state, action) + assert.equal(menuBuildFromTemplateSpy.calledOnce, false) + assert.equal(menuPopupSpy.calledOnce, false) + }) + + it('index is correct', function () { + const action = { + actionType: windowConstants.WINDOW_ON_TAB_PAGE_MENU, + index: 0 + } + contextMenuReducer(state, action) + assert.equal(menuBuildFromTemplateSpy.calledOnce, true) + assert.equal(menuPopupSpy.calledOnce, true) + }) + }) +})