From 4d6fe06e0e78463a098b9c0099c41f5171f17158 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Fri, 6 Oct 2017 00:40:23 -0400 Subject: [PATCH] 18 Unbelievable ways to get rid of dead tabs on tab close Fix #11028 Fix #10696 Fix #10905 Fix #9420 --- app/browser/reducers/tabsReducer.js | 24 ++++- app/browser/tabs.js | 96 ++++++++++++++++++- app/renderer/components/main/main.js | 4 - app/renderer/reducers/contextMenuReducer.js | 6 +- app/renderer/reducers/frameReducer.js | 70 +++++--------- js/actions/appActions.js | 45 +++++++++ js/actions/windowActions.js | 15 --- js/constants/appConstants.js | 4 + js/constants/messages.js | 1 - js/constants/windowConstants.js | 1 - js/contextMenus.js | 14 +-- js/stores/windowStore.js | 5 +- test/lib/brave.js | 37 +++++++ test/tab-components/pinnedTabTest.js | 35 +++++++ test/tab-components/tabPagesTest.js | 29 +++--- test/tab-components/tabTest.js | 39 +++++++- .../app/browser/reducers/tabsReducerTest.js | 12 ++- test/unit/js/stores/windowStoreTest.js | 4 - 18 files changed, 330 insertions(+), 111 deletions(-) diff --git a/app/browser/reducers/tabsReducer.js b/app/browser/reducers/tabsReducer.js index 60f7142e2b9..23df4bdb537 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,7 +168,7 @@ const tabsReducer = (state, action, immutableAction) => { tabs.toggleDevTools(tabId) }) } else { - // This check is only needed because sometimes front end can try to close + // 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) { @@ -190,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 b986e2b4c81..cbb129d05a0 100644 --- a/app/browser/tabs.js +++ b/app/browser/tabs.js @@ -430,11 +430,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 @@ -526,6 +521,14 @@ const api = { windowActions.gotResponseDetails(tabId, {status, newURL, originalURL, httpResponseCode, requestMethod, referrer, headers, 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) => { @@ -985,6 +988,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() @@ -1042,6 +1125,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 2936172942b..4a4bfb0aacd 100644 --- a/app/renderer/components/main/main.js +++ b/app/renderer/components/main/main.js @@ -373,10 +373,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 5248ddb5810..8cbe6cdb6af 100644 --- a/app/renderer/reducers/contextMenuReducer.js +++ b/app/renderer/reducers/contextMenuReducer.js @@ -38,7 +38,7 @@ const urlUtil = require('../../../js/lib/urlutil') const frameStateUtil = require('../../../js/state/frameStateUtil') const dndData = require('../../../js/dndData') 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) { @@ -93,9 +93,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 be38196463d..dc79b370feb 100644 --- a/app/renderer/reducers/frameReducer.js +++ b/app/renderer/reducers/frameReducer.js @@ -79,22 +79,40 @@ 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. @@ -156,22 +174,10 @@ 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) - - // 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 - - if (!isNewTab && activeTabHasUpdated) { - state = frameStateUtil.updateTabPageIndex(state, tabId) - state = state.set('previewFrameKey', null) - } - } + 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) } break case windowConstants.WINDOW_SET_NAVIGATED: @@ -224,34 +230,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 130820c22e2..c25ea94998f 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -155,6 +155,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 be8f8aa9e94..f51781ae411 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 030fa0235ab..a71d5439148 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -60,6 +60,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 21afa547b8a..7eb32375357 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 002f927062f..fa7e3e214a9 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -100,7 +100,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 784b46126aa..276230c2f3e 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -483,24 +483,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 83ec6000aec..e62d3c2df9f 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -158,10 +158,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 6fe83abb0a6..9ac0f3e969d 100644 --- a/test/lib/brave.js +++ b/test/lib/brave.js @@ -618,6 +618,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 95bf04d8927..8ea76177d98 100644 --- a/test/tab-components/pinnedTabTest.js +++ b/test/tab-components/pinnedTabTest.js @@ -74,6 +74,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 * () { @@ -177,6 +211,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 43653cecad0..242845c25b3 100644 --- a/test/tab-components/tabTest.js +++ b/test/tab-components/tabTest.js @@ -266,6 +266,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 @@ -304,7 +341,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 b139f7b782b..c4172584009 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 6f8e5a294e1..5b60c95c3a7 100644 --- a/test/unit/js/stores/windowStoreTest.js +++ b/test/unit/js/stores/windowStoreTest.js @@ -268,10 +268,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 () {