diff --git a/app/browser/reducers/tabsReducer.js b/app/browser/reducers/tabsReducer.js index 09af8ef7139..c3c09c024f3 100644 --- a/app/browser/reducers/tabsReducer.js +++ b/app/browser/reducers/tabsReducer.js @@ -98,6 +98,27 @@ const tabsReducer = (state, action, immutableAction) => { }) break } + case appConstants.APP_TAB_PAGE_CLOSE_MENU_ITEM_CLICKED: { + const windowId = action.get('windowId') + const tabPageIndex = action.get('tabPageIndex') + state = tabs.closeTabPage(state, windowId, tabPageIndex) + break + } + case appConstants.APP_CLOSE_TABS_TO_LEFT_MENU_ITEM_CLICKED: { + const tabId = action.get('tabId') + state = tabs.closeTabsToLeft(state, tabId) + break + } + case appConstants.APP_CLOSE_TABS_TO_RIGHT_MENU_ITEM_CLICKED: { + const tabId = action.get('tabId') + state = tabs.closeTabsToRight(state, tabId) + break + } + case appConstants.APP_CLOSE_OTHER_TABS_MENU_ITEM_CLICKED: { + const tabId = action.get('tabId') + state = tabs.closeOtherTabs(state, tabId) + break + } case appConstants.APP_CREATE_TAB_REQUESTED: if (action.getIn(['createProperties', 'windowId']) == null) { const senderWindowId = action.getIn(['senderWindowId']) @@ -147,6 +168,12 @@ const tabsReducer = (state, action, immutableAction) => { tabs.toggleDevTools(tabId) }) } else { + // This check is only needed in case front end tries to close + // a tabId it thinks exists but doesn't actually exist anymore. + const tabValue = tabState.getByTabId(state, tabId) + if (!tabValue) { + break + } const windowId = tabState.getWindowId(state, tabId) const nonPinnedTabs = tabState.getNonPinnedTabsByWindowId(state, windowId) const pinnedTabs = tabState.getPinnedTabsByWindowId(state, windowId) @@ -184,6 +211,7 @@ const tabsReducer = (state, action, immutableAction) => { tabs.setActive(nextActiveTabId) } }) + tabs.forgetTab(tabId) } break case appConstants.APP_ALLOW_FLASH_ONCE: diff --git a/app/browser/tabs.js b/app/browser/tabs.js index 289fe974b70..ab165955db1 100644 --- a/app/browser/tabs.js +++ b/app/browser/tabs.js @@ -452,11 +452,6 @@ const api = { updateTab(tabId, changeInfo) }) - process.on('chrome-tabs-removed', (tabId, windowId) => { - appActions.tabClosed(tabId, windowId) - cleanupWebContents(tabId) - }) - app.on('web-contents-created', function (event, tab) { if (tab.isBackgroundPage() || !tab.isGuest()) { return @@ -548,6 +543,14 @@ const api = { windowActions.gotResponseDetails(tabId, {status, newURL, originalURL, httpResponseCode, requestMethod, referrer, resourceType}) } }) + + tab.once('will-destroy', () => { + const tabValue = getTabValue(tabId) + if (tabValue) { + const windowId = tabValue.get('windowId') + appActions.tabClosed(tabId, windowId) + } + }) }) process.on('on-tab-created', (tab, options) => { @@ -1019,6 +1022,86 @@ const api = { return nextTabId }, + + closeTabPage: (state, windowId, tabPageIndex) => { + const tabsPerPage = Number(getSetting(settings.TABS_PER_PAGE)) + const startTabIndex = tabPageIndex * tabsPerPage + const pinnedTabsCount = tabState.getPinnedTabsByWindowId(state, windowId).size + tabState.getTabsByWindowId(state, windowId) + .sort((tab1, tab2) => tab1.get('index') - tab2.get('index')) + .filter((tabValue) => !tabValue.get('pinned')) + .slice(startTabIndex + pinnedTabsCount, startTabIndex + tabsPerPage + pinnedTabsCount) + .forEach((tabValue) => { + const tab = getWebContents(tabValue.get('tabId')) + if (tab && !tab.isDestroyed()) { + tab.forceClose() + } + }) + state = api.updateTabsStateForWindow(state, windowId) + return state + }, + + closeTabsToLeft: (state, tabId) => { + const tabValue = tabState.getByTabId(state, tabId) + if (!tabValue) { + return state + } + const index = tabValue.get('index') + const windowId = tabValue.get('windowId') + const pinnedTabsCount = tabState.getPinnedTabsByWindowId(state, windowId).size + tabState.getTabsByWindowId(state, windowId) + .sort((tab1, tab2) => tab1.get('index') - tab2.get('index')) + .filter((tabValue) => !tabValue.get('pinned')) + .slice(0, index - pinnedTabsCount) + .forEach((tabValue) => { + const tab = getWebContents(tabValue.get('tabId')) + if (tab && !tab.isDestroyed()) { + tab.forceClose() + } + }) + state = api.updateTabsStateForWindow(state, windowId) + return state + }, + + closeTabsToRight: (state, tabId) => { + const tabValue = tabState.getByTabId(state, tabId) + if (!tabValue) { + return state + } + const index = tabValue.get('index') + const windowId = tabValue.get('windowId') + const pinnedTabsCount = tabState.getPinnedTabsByWindowId(state, windowId).size + tabState.getTabsByWindowId(state, windowId) + .sort((tab1, tab2) => tab1.get('index') - tab2.get('index')) + .filter((tabValue) => !tabValue.get('pinned')) + .slice(index + 1 - pinnedTabsCount) + .forEach((tabValue) => { + const tab = getWebContents(tabValue.get('tabId')) + if (tab && !tab.isDestroyed()) { + tab.forceClose() + } + }) + state = api.updateTabsStateForWindow(state, windowId) + return state + }, + + closeOtherTabs: (state, tabId) => { + const tabValue = tabState.getByTabId(state, tabId) + if (!tabValue) { + return state + } + const windowId = tabValue.get('windowId') + tabState.getTabsByWindowId(state, windowId) + .forEach((tabValue) => { + const tab = getWebContents(tabValue.get('tabId')) + if (tab && !tab.isDestroyed() && tabValue.get('tabId') !== tabId && !tabValue.get('pinned')) { + tab.forceClose() + } + }) + state = api.updateTabsStateForWindow(state, windowId) + return state + }, + debugTabs: (state) => { console.log(tabState.getTabs(state) .toJS() @@ -1076,6 +1159,9 @@ const api = { } }) return state + }, + forgetTab: (tabId) => { + cleanupWebContents(tabId) } } diff --git a/app/renderer/components/main/main.js b/app/renderer/components/main/main.js index a637991a3a9..5b1a56bb085 100644 --- a/app/renderer/components/main/main.js +++ b/app/renderer/components/main/main.js @@ -393,10 +393,6 @@ class Main extends React.Component { ipc.on(messages.SHORTCUT_UNDO_CLOSED_FRAME, () => windowActions.undoClosedFrame()) - ipc.on(messages.SHORTCUT_CLOSE_OTHER_FRAMES, (e, tabId, isCloseRight, isCloseLeft) => { - windowActions.closeOtherFrames(tabId, isCloseRight, isCloseLeft) - }) - ipc.on(messages.SHOW_DOWNLOADS_TOOLBAR, () => { windowActions.setDownloadsToolbarVisible(true) }) diff --git a/app/renderer/reducers/contextMenuReducer.js b/app/renderer/reducers/contextMenuReducer.js index 1896c5629e6..4f6c02b3f46 100644 --- a/app/renderer/reducers/contextMenuReducer.js +++ b/app/renderer/reducers/contextMenuReducer.js @@ -40,7 +40,7 @@ const dndData = require('../../../js/dndData') const bookmarkFoldersUtil = require('../../common/lib/bookmarkFoldersUtil') const historyState = require('../../common/state/historyState') const {makeImmutable, isMap} = require('../../common/state/immutableUtil') -const {getCurrentWindow} = require('../../renderer/currentWindow') +const {getCurrentWindow, getCurrentWindowId} = require('../../renderer/currentWindow') const {getSetting} = require('../../../js/settings') const validateAction = function (action) { @@ -95,9 +95,7 @@ const onTabPageMenu = function (state, action) { }, { label: locale.translation('closeTabPage'), click: () => { - tabPageFrames - .map((frame) => frame.get('tabId')) - .forEach((tabId) => appActions.tabCloseRequested(tabId)) + appActions.tabPageCloseMenuItemClicked(getCurrentWindowId(), index) } }] diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js index 5a484296779..19ab4e6ea93 100644 --- a/app/renderer/reducers/frameReducer.js +++ b/app/renderer/reducers/frameReducer.js @@ -80,22 +80,41 @@ const getLocation = (location) => { const frameReducer = (state, action, immutableAction) => { switch (action.actionType) { + case appConstants.APP_TAB_CLOSED: { + const tabId = immutableAction.get('tabId') + const frame = frameStateUtil.getFrameByTabId(state, tabId) + if (frame) { + action.frameKey = frame.get('key') + state = closeFrame(state, action) + const activeFrame = frameStateUtil.getActiveFrame(state) + if (activeFrame) { + state = frameStateUtil.updateTabPageIndex(state, activeFrame.get('tabId')) + } + } + break + } case appConstants.APP_TAB_UPDATED: // This case will be fired for both tab creation and tab update. - // being `tabValue` set for tab creation and `changeInfo` set for tab update const tab = immutableAction.get('tabValue') const changeInfo = immutableAction.get('changeInfo') if (!tab) { break } const tabId = tab.get('tabId') + if (tabId === -1) { + break + } let frame = frameStateUtil.getFrameByTabId(state, tabId) if (!frame) { break } let frames = state.get('frames') const index = tab.get('index') + // If front end doesn't know about this tabId, then do nothing! let sourceFrameIndex = frameStateUtil.getIndexByTabId(state, tabId) + if (sourceFrameIndex == null) { + break + } if (index != null && sourceFrameIndex !== index && // Only update the index once the frame is known. @@ -147,29 +166,26 @@ const frameReducer = (state, action, immutableAction) => { } const active = tab.get('active') - if (active != null) { - if (active) { - state = frameStateUtil.setActiveFrameKey(state, frame.get('key')) - state = frameStateUtil.setFrameLastAccessedTime(state, sourceFrameIndex) + if (active && state.get('activeFrameKey') !== frame.get('key')) { + state = frameStateUtil.setActiveFrameKey(state, frame.get('key')) + state = frameStateUtil.setFrameLastAccessedTime(state, sourceFrameIndex) + state = state.set('previewFrameKey', null) - // Handle tabPage updates and preview cancelation based on tab updated - // otherwise tabValue will fire those events each time a tab finish loading - // see bug #8429 - const isNewTab = changeInfo.isEmpty() - const activeTabHasUpdated = changeInfo.get('active') != null - const hasBeenActivated = frame.get('hasBeenActivated') + // Handle tabPage updates and preview cancelation based on tab updated + // otherwise tabValue will fire those events each time a tab finish loading + // see bug #8429 + const isNewTab = changeInfo.isEmpty() + const activeTabHasUpdated = changeInfo.get('active') != null + const hasBeenActivated = frame.get('hasBeenActivated') - if (!isNewTab && activeTabHasUpdated) { - state = frameStateUtil.updateTabPageIndex(state, tabId) - state = state.set('previewFrameKey', null) - // Show the phishing warning if we are showing a data: tab - // for the first time - state = state.setIn(['ui', 'siteInfo', 'isVisible'], - !hasBeenActivated && isPotentialPhishingUrl(tab.get('url'))) - } - if (!frame.get('hasBeenActivated')) { - state = state.setIn(['frames', index, 'hasBeenActivated'], true) - } + if (!isNewTab && activeTabHasUpdated) { + // Show the phishing warning if we are showing a data: tab + // for the first time + state = state.setIn(['ui', 'siteInfo', 'isVisible'], + !hasBeenActivated && isPotentialPhishingUrl(tab.get('url'))) + } + if (!frame.get('hasBeenActivated')) { + state = state.setIn(['frames', index, 'hasBeenActivated'], true) } } break @@ -220,34 +236,6 @@ const frameReducer = (state, action, immutableAction) => { appActions.tabCloseRequested(frame.get('tabId')) }) break - case windowConstants.WINDOW_CLOSE_OTHER_FRAMES: - { - const currentIndex = frameStateUtil.getIndexByTabId(state, action.tabId) - if (currentIndex === -1) { - break - } - - let tabs = [] - - state.get('frames').forEach((frame, i) => { - if (!frame.get('pinnedLocation') && - ((i < currentIndex && action.isCloseLeft) || (i > currentIndex && action.isCloseRight))) { - if (frame) { - tabs.push(frame.get('tabId')) - appActions.tabCloseRequested(frame.get('tabId')) - } - } - }) - - // TODO(nejc) this can be simplified when states are merged - const newFrames = state.get('frames').filter(frame => !tabs.includes(frame.get('tabId'))) - let newState = state.set('frames', newFrames) - newState = frameStateUtil.updateTabPageIndex(newState, action.tabId) - const index = newState.getIn(['ui', 'tabs', 'tabPageIndex'], 0) - state = state.setIn(['ui', 'tabs', 'tabPageIndex'], index) - } - break - case windowConstants.WINDOW_CLOSE_FRAME: state = closeFrame(state, action) const activeFrame = frameStateUtil.getActiveFrame(state) diff --git a/js/actions/appActions.js b/js/actions/appActions.js index dd55f774036..55f010b45de 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -166,6 +166,51 @@ const appActions = { }) }, + /** + * Menu item for closing a tab page has been clicked. + * @param {Number} tabPageIndex The index of the tab page to close + */ + tabPageCloseMenuItemClicked: function (windowId, tabPageIndex) { + dispatch({ + actionType: appConstants.APP_TAB_PAGE_CLOSE_MENU_ITEM_CLICKED, + tabPageIndex, + windowId + }) + }, + + /** + * Menu item for closing tabs to the left has been clicked. + * @param {Number} tabId The tabId woh's tabs to the left should be closed. + */ + closeTabsToLeftMenuItemClicked: function (tabId) { + dispatch({ + actionType: appConstants.APP_CLOSE_TABS_TO_LEFT_MENU_ITEM_CLICKED, + tabId + }) + }, + + /** + * Menu item for closing tabs to the right has been clicked. + * @param {Number} tabId The tabId woh's tabs to the right should be closed. + */ + closeTabsToRightMenuItemClicked: function (tabId) { + dispatch({ + actionType: appConstants.APP_CLOSE_TABS_TO_RIGHT_MENU_ITEM_CLICKED, + tabId + }) + }, + + /** + * Menu item for closing other tabs than the specified tab. + * @param {Number} tabId The tabId woh's tabs to the left should be closed. + */ + closeOtherTabsMenuItemClicked: function (tabId) { + dispatch({ + actionType: appConstants.APP_CLOSE_OTHER_TABS_MENU_ITEM_CLICKED, + tabId + }) + }, + /** * A request for a new tab has been made with the specified createProperties * @param {Object} createProperties diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index b25af0c3b7e..0e40c95519a 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -187,21 +187,6 @@ const windowActions = { }) }, - /** - * Dispatches a message to close multiple frames - * @param {string} tabId - Frame that we want to ignore when closing all tabs - * @param {boolean} isCloseRight - Close frames to the right of the frame provided - * @param {boolean} isCloseLeft - Close frames to the left of the frame provided - */ - closeOtherFrames: function (tabId, isCloseRight, isCloseLeft) { - dispatch({ - actionType: windowConstants.WINDOW_CLOSE_OTHER_FRAMES, - tabId, - isCloseRight, - isCloseLeft - }) - }, - /** * Dispatches a message to the store to undo a closed frame * The new frame is expected to appear at the index it was last closed at diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index c4e15ef19d6..2c86c12f25f 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -52,6 +52,10 @@ const appConstants = { APP_TAB_CREATED: _, APP_TAB_MOVED: _, APP_TAB_DETACH_MENU_ITEM_CLICKED: _, + APP_TAB_PAGE_CLOSE_MENU_ITEM_CLICKED: _, + APP_CLOSE_TABS_TO_LEFT_MENU_ITEM_CLICKED: _, + APP_CLOSE_TABS_TO_RIGHT_MENU_ITEM_CLICKED: _, + APP_CLOSE_OTHER_TABS_MENU_ITEM_CLICKED: _, APP_TAB_ATTACHED: _, APP_TAB_WILL_ATTACH: _, APP_TAB_ACTIVATE_REQUESTED: _, diff --git a/js/constants/messages.js b/js/constants/messages.js index c72ace77b15..bd714c146c6 100644 --- a/js/constants/messages.js +++ b/js/constants/messages.js @@ -35,7 +35,6 @@ const messages = { SHORTCUT_ACTIVE_FRAME_FIND_PREV: _, // Frame management shortcuts SHORTCUT_CLOSE_FRAME: _, /** @arg {number} opt_key of frame, defaults to active frame */ - SHORTCUT_CLOSE_OTHER_FRAMES: _, /** @arg {boolean} close to the right, @arg {boolean} close to the left */ SHORTCUT_UNDO_CLOSED_FRAME: _, SHORTCUT_FRAME_MUTE: _, SHORTCUT_FRAME_RELOAD: _, /** @arg {number} key of frame */ diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 141ae5eaeea..79267535ca9 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -94,7 +94,6 @@ const windowConstants = { WINDOW_SET_ALL_AUDIO_MUTED: _, WINDOW_ON_GO_BACK_LONG: _, WINDOW_ON_GO_FORWARD_LONG: _, - WINDOW_CLOSE_OTHER_FRAMES: _, WINDOW_ON_CERT_ERROR: _, WINDOW_ON_TAB_PAGE_CONTEXT_MENU: _, WINDOW_ON_FRAME_BOOKMARK: _, diff --git a/js/contextMenus.js b/js/contextMenus.js index 0777e1f6dc3..347d9ab6927 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -531,24 +531,18 @@ function tabTemplateInit (frameProps) { template.push({ label: locale.translation('closeOtherTabs'), - click: (item, focusedWindow) => { - if (focusedWindow) { - focusedWindow.webContents.send(messages.SHORTCUT_CLOSE_OTHER_FRAMES, tabId, true, true) - } + click: (item) => { + appActions.closeOtherTabsMenuItemClicked(tabId) } }, { label: locale.translation('closeTabsToRight'), click: (item, focusedWindow) => { - if (focusedWindow) { - focusedWindow.webContents.send(messages.SHORTCUT_CLOSE_OTHER_FRAMES, tabId, true, false) - } + appActions.closeTabsToRightMenuItemClicked(tabId) } }, { label: locale.translation('closeTabsToLeft'), click: (item, focusedWindow) => { - if (focusedWindow) { - focusedWindow.webContents.send(messages.SHORTCUT_CLOSE_OTHER_FRAMES, tabId, false, true) - } + appActions.closeTabsToLeftMenuItemClicked(tabId) } }, CommonMenu.separatorMenuItem) diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 7df5505bdb1..9cbe765e8e4 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -160,10 +160,7 @@ const newFrame = (state, frameOpts) => { const tabId = frameOpts.tabId const frame = frameStateUtil.getFrameByTabId(state, tabId) state = frameStateUtil.updateTabPageIndex(state, tabId) - if (active && frame) { - // only set the activeFrameKey if the tab is already active - state = state.set('activeFrameKey', frame.get('key')) - } else { + if (!active || !frame) { appActions.tabActivateRequested(tabId) } } diff --git a/test/lib/brave.js b/test/lib/brave.js index 31f02bc9a3f..c8a343003e6 100644 --- a/test/lib/brave.js +++ b/test/lib/brave.js @@ -592,6 +592,43 @@ var exports = { }) }) + this.app.client.addCommand('closeTabPageByIndex', function (tabPageIndex, windowId = -1) { + logVerbose('closeTabPageByIndex("' + windowId + '", "' + tabPageIndex + '")') + return this.execute(function (windowId, tabPageIndex) { + return devTools('appActions').tabPageCloseMenuItemClicked(windowId, tabPageIndex) + }, windowId, tabPageIndex).then((response) => response.value) + }) + + this.app.client.addCommand('closeTabsToLeft', function (index) { + logVerbose('closeTabsToLeft(' + index + ')') + return this.waitForTab({index}).getAppState().then((val) => { + const tab = val.value.tabs.find((tab) => tab.index === index) + return this.execute(function (tabId) { + devTools('appActions').closeTabsToLeftMenuItemClicked(tabId) + }, tab.tabId) + }) + }) + + this.app.client.addCommand('closeTabsToRight', function (index) { + logVerbose('closeTabsToRight(' + index + ')') + return this.waitForTab({index}).getAppState().then((val) => { + const tab = val.value.tabs.find((tab) => tab.index === index) + return this.execute(function (tabId) { + devTools('appActions').closeTabsToRightMenuItemClicked(tabId) + }, tab.tabId) + }) + }) + + this.app.client.addCommand('closeOtherTabs', function (index) { + logVerbose('closeOtherTabs(' + index + ')') + return this.waitForTab({index}).getAppState().then((val) => { + const tab = val.value.tabs.find((tab) => tab.index === index) + return this.execute(function (tabId) { + devTools('appActions').closeOtherTabsMenuItemClicked(tabId) + }, tab.tabId) + }) + }) + this.app.client.addCommand('closeTabByIndex', function (index) { return this.waitForTab({index}).getAppState().then((val) => { const tab = val.value.tabs.find((tab) => tab.index === index) diff --git a/test/tab-components/pinnedTabTest.js b/test/tab-components/pinnedTabTest.js index 8f05dcba4e8..970ae2b4a40 100644 --- a/test/tab-components/pinnedTabTest.js +++ b/test/tab-components/pinnedTabTest.js @@ -127,6 +127,40 @@ describe('pinnedTabs', function () { }) }) + describe('close groups of tab with some pinned', function () { + Brave.beforeEach(this) + beforeEach(function * () { + this.page1 = Brave.server.url('page1.html') + yield setup(this.app.client) + this.app.client + .newTab({pinned: true}) + .waitForElementCount('[data-test-id="tab"]', 2) + .newTab() + .waitForElementCount('[data-test-id="tab"]', 3) + }) + describe('closeTabsToRightMenuItemClicked', function () { + it('can close tabs to the right', function * () { + yield this.app.client + .closeTabsToRight(1) + .waitForElementCount('[data-test-id="tab"]', 2) + }) + }) + describe('closeTabsToLeftMenuItemClicked', function () { + it('can close tabs to the left', function * () { + yield this.app.client + .closeTabsToLeft(2) + .waitForElementCount('[data-test-id="tab"]', 2) + }) + }) + describe('closeOtherTabsMenuItemClicked', function () { + it('can close other tabs', function * () { + yield this.app.client + .closeOtherTabs(2) + .waitForElementCount('[data-test-id="tab"]', 2) + }) + }) + }) + describe('gets pins from external windows', function () { Brave.beforeEach(this) beforeEach(function * () { @@ -231,6 +265,7 @@ describe('pinnedTabs', function () { it('navigating to a different origin opens a new tab', function * () { const page2 = Brave.server.url('page2.html').replace('localhost', '127.0.0.1') yield this.app.client + .activateURLMode() .click(urlInput) .setValue(urlInput, page2) .keys(Brave.keys.ENTER) diff --git a/test/tab-components/tabPagesTest.js b/test/tab-components/tabPagesTest.js index 162557885d6..617e9284f4a 100644 --- a/test/tab-components/tabPagesTest.js +++ b/test/tab-components/tabPagesTest.js @@ -3,7 +3,6 @@ const Brave = require('../lib/brave') const appConfig = require('../../js/constants/appConfig') const settings = require('../../js/constants/settings') -const messages = require('../../js/constants/messages') const { urlInput, newFrameButton, @@ -97,23 +96,25 @@ describe('tab pages', function () { .waitForElementCount(tabsTabs, numTabsPerPage) }) - it('closing tabs with close-to-left option', function * () { - let tabId = 0 + it('closing tab page option for non active tab page', function * () { yield this.app.client .click(newFrameButton) .waitForElementCount(tabPage, 2) - .waitUntil(function () { - return this.getAppState().then((state) => { - const length = state.value.tabs.length - tabId = state.value.tabs[length - 1].id - return true - }) - }) - .waitUntil(function () { - return this.ipcSend(messages.SHORTCUT_CLOSE_OTHER_FRAMES, tabId, false, true) - }) + .closeTabPageByIndex(0, 1) .waitForElementCount(tabPage, 0) - .waitForElementCount(tabsTabs, 1) + .waitForElementCount('[data-test-id="tab"]', 1) + .waitForVisible('[data-test-active-tab]') + }) + + it('closing tab page option for active tab page', function * () { + const numTabsPerPage = appConfig.defaultSettings[settings.TABS_PER_PAGE] + yield this.app.client + .click(newFrameButton) + .waitForElementCount(tabPage, 2) + .closeTabPageByIndex(1, 1) + .waitForElementCount(tabPage, 0) + .waitForElementCount('[data-test-id="tab"]', numTabsPerPage) + .waitForVisible('[data-test-active-tab]') }) describe('allows changing to tab pages', function () { diff --git a/test/tab-components/tabTest.js b/test/tab-components/tabTest.js index b3bd13c1718..b009d696293 100644 --- a/test/tab-components/tabTest.js +++ b/test/tab-components/tabTest.js @@ -267,6 +267,43 @@ describe('tab tests', function () { }) }) + describe('close group of tabs', function () { + Brave.beforeEach(this) + beforeEach(function * () { + this.page1 = Brave.server.url('page1.html') + yield setup(this.app.client) + this.app.client + .newTab() + .waitForElementCount('[data-test-id="tab"]', 2) + .newTab() + .waitForElementCount('[data-test-id="tab"]', 3) + }) + describe('closeTabsToRightMenuItemClicked', function () { + it('can close tabs to the right', function * () { + yield this.app.client + .closeTabsToRight(0) + .waitForElementCount('[data-test-id="tab"]', 1) + .waitForExist('[data-test-active-tab][data-frame-key="1"]') + }) + }) + describe('closeTabsToLeftMenuItemClicked', function () { + it('can close tabs to the left', function * () { + yield this.app.client + .closeTabsToLeft(2) + .waitForElementCount('[data-test-id="tab"]', 1) + .waitForExist('[data-test-active-tab][data-frame-key="3"]') + }) + }) + describe('closeOtherTabsMenuItemClicked', function () { + it('can close other tabs', function * () { + yield this.app.client + .closeOtherTabs(0) + .waitForElementCount('[data-test-id="tab"]', 1) + .waitForExist('[data-test-active-tab][data-frame-key="1"]') + }) + }) + }) + describe('close tab', function () { const tabCountBeforeTabClose = 2 const tabCountAfterTabClose = 1 @@ -305,7 +342,7 @@ describe('tab tests', function () { // This ensures it's actually unloaded .waitForTabCount(1) .windowByUrl(Brave.browserWindowUrl) - .ipcSend(messages.SHORTCUT_CLOSE_OTHER_FRAMES, 1, true, true) + .closeOtherTabs(0) .waitForTabCount(tabCountAfterTabClose) }) it('should undo last closed tab', function * () { diff --git a/test/unit/app/browser/reducers/tabsReducerTest.js b/test/unit/app/browser/reducers/tabsReducerTest.js index be53e402f7c..cf59fd9c4d7 100644 --- a/test/unit/app/browser/reducers/tabsReducerTest.js +++ b/test/unit/app/browser/reducers/tabsReducerTest.js @@ -90,7 +90,8 @@ describe('tabsReducer unit tests', function () { moveTo: sinon.mock(), reload: sinon.mock(), updateTabsStateForWindow: sinon.mock(), - create: sinon.mock() + create: sinon.mock(), + forgetTab: sinon.spy() } this.windowsAPI = require('../../../../../app/browser/windows') @@ -267,6 +268,7 @@ describe('tabsReducer unit tests', function () { beforeEach(function () { this.removeTabByTabIdSpy = sinon.stub(this.tabStateAPI, 'removeTabByTabId', (state) => state) this.tabsAPI.setActive.reset() + this.tabsAPI.forgetTab.reset() }) afterEach(function () { @@ -278,6 +280,7 @@ describe('tabsReducer unit tests', function () { tabsReducer(this.state, action) assert.equal(this.tabStateAPI.removeTabByTabId.getCall(0).args[1], action.tabId) assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2) + assert(this.tabsAPI.forgetTab.withArgs(5).calledOnce) }) it('does nothing if tabId is TAB_ID_NONE', function () { @@ -303,6 +306,7 @@ describe('tabsReducer unit tests', function () { this.clock.tick(1510) assert(this.tabsAPI.setActive.withArgs(3).calledOnce) assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2) + assert(this.tabsAPI.forgetTab.withArgs(5).calledOnce) }) }) @@ -327,6 +331,7 @@ describe('tabsReducer unit tests', function () { this.clock.tick(1510) assert(this.tabsAPI.setActive.withArgs(4).calledOnce) assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2) + assert(this.tabsAPI.forgetTab.withArgs(3).calledOnce) }) it('chooses next unpinned tab', function () { @@ -342,6 +347,7 @@ describe('tabsReducer unit tests', function () { this.clock.tick(1510) assert(this.tabsAPI.setActive.withArgs(5).calledOnce) assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2) + assert(this.tabsAPI.forgetTab.withArgs(3).calledOnce) }) it('chooses previous unpinned tab', function () { @@ -353,6 +359,7 @@ describe('tabsReducer unit tests', function () { this.clock.tick(1510) assert(this.tabsAPI.setActive.withArgs(3).calledOnce) assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2) + assert(this.tabsAPI.forgetTab.withArgs(5).calledOnce) }) describe('if no unpinned tabs come after this', function () { @@ -370,6 +377,7 @@ describe('tabsReducer unit tests', function () { this.clock.tick(1510) assert(this.tabsAPI.setActive.withArgs(4).calledOnce) assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2) + assert(this.tabsAPI.forgetTab.withArgs(3).calledOnce) }) }) }) @@ -380,6 +388,7 @@ describe('tabsReducer unit tests', function () { this.clock.tick(1510) assert(this.tabsAPI.setActive.withArgs(4).calledOnce) assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2) + assert(this.tabsAPI.forgetTab.withArgs(5).calledOnce) }) it('chooses parent tab id (even if parent tab was NOT last active)', function () { @@ -390,6 +399,7 @@ describe('tabsReducer unit tests', function () { this.clock.tick(1510) assert(this.tabsAPI.setActive.withArgs(3).calledOnce) assert.equal(this.tabsAPI.updateTabsStateForWindow.getCall(0).args[1], 2) + assert(this.tabsAPI.forgetTab.withArgs(5).calledOnce) }) }) }) diff --git a/test/unit/js/stores/windowStoreTest.js b/test/unit/js/stores/windowStoreTest.js index 7538230b7e5..2dd760d8d06 100644 --- a/test/unit/js/stores/windowStoreTest.js +++ b/test/unit/js/stores/windowStoreTest.js @@ -267,10 +267,6 @@ describe('Window store unit tests', function () { windowStore = require('../../../../js/stores/windowStore.js') windowState = windowStore.getState() }) - - it('sets activeFrameKey', function () { - assert(windowState.get('activeFrameKey')) - }) }) describe('when tab being opened is not active', function () {