diff --git a/app/browser/activeTabHistory.js b/app/browser/activeTabHistory.js index a3650face01..9569c9a5197 100644 --- a/app/browser/activeTabHistory.js +++ b/app/browser/activeTabHistory.js @@ -55,6 +55,18 @@ const api = { */ clearTabbedWindow: function (windowId) { activeTabsByWindow.delete(windowId) + }, + + /** + * Replace all intances of `oldTabId` with `newTabId` in + * active tab history for each window + */ + tabIdChanged: function (oldTabId, newTabId) { + for (const [windowId, windowActiveTabs] of activeTabsByWindow) { + if (windowActiveTabs && windowActiveTabs.length) { + activeTabsByWindow.set(windowId, windowActiveTabs.map(previousTabId => (previousTabId === oldTabId) ? newTabId : previousTabId)) + } + } } } diff --git a/app/browser/reducers/tabsReducer.js b/app/browser/reducers/tabsReducer.js index b2935dbffba..0b4e68c3223 100644 --- a/app/browser/reducers/tabsReducer.js +++ b/app/browser/reducers/tabsReducer.js @@ -150,6 +150,9 @@ const tabsReducer = (state, action, immutableAction) => { state = tabState.maybeCreateTab(state, action) // tabs.debugTabs(state) break + case appConstants.APP_TAB_REPLACED: + tabs.tabIdChanged(action.get('oldTabId'), action.get('newTabId')) + break case appConstants.APP_TAB_CLOSE_REQUESTED: { let tabId = action.get('tabId') diff --git a/app/browser/tabs.js b/app/browser/tabs.js index b5e0da8b6c4..cf089f2cbc4 100644 --- a/app/browser/tabs.js +++ b/app/browser/tabs.js @@ -458,7 +458,10 @@ const api = { } const tabId = newTab.getId() + + // update our webContents Map with the openerTabId webContentsCache.updateWebContents(tabId, newTab, openerTabId) + let newTabValue = getTabValue(newTab.getId()) let windowId @@ -526,6 +529,10 @@ const api = { } const tabId = tab.getId() + // This is the first event for WebContents so cache the item in the Map + // This is not ideal since we do not know the old tabId here, we cannot + webContentsCache.updateWebContents(tabId, tab, null) + // command-line flag --debug-tab-events if (shouldDebugTabEvents) { console.log(`Tab [${tabId}] created in window ${tab.tabValue().windowId}`) @@ -847,6 +854,14 @@ const api = { } }, + tabIdChanged: (oldTabId, newTabId) => { + if (shouldDebugTabEvents) { + console.log(`Tab [${oldTabId}] changed to tabId ${newTabId}. Updating state references...`) + } + webContentsCache.tabIdChanged(oldTabId, newTabId) + activeTabHistory.tabIdChanged(oldTabId, newTabId) + }, + pin: (state, tabId, pinned) => { const tab = webContentsCache.getWebContents(tabId) if (tab && !tab.isDestroyed()) { @@ -998,6 +1013,9 @@ const api = { }, moveTo: (state, tabId, frameOpts, browserOpts, toWindowId) => { + if (shouldDebugTabEvents) { + console.log(`tabs.moveTo: tab:${tabId} to window:${toWindowId}`) + } frameOpts = makeImmutable(frameOpts) browserOpts = makeImmutable(browserOpts) const tab = webContentsCache.getWebContents(tabId) @@ -1027,8 +1045,13 @@ const api = { return } - // detach from current window - tab.detach(() => { + if (shouldDebugTabEvents) { + console.log(`moving tab ${tabId} so first detaching it`) + } + // after tab has been removed from initial window, + // set active tab in old window, and + // move tab to new window + const attachToNewWindow = () => { // handle tab has detached from window // handle tab was the active tab of the window if (tabValue.get('active')) { @@ -1039,6 +1062,9 @@ const api = { } } if (toWindowId == null || toWindowId === -1) { + if (shouldDebugTabEvents) { + console.log('creating new window for moved tab') + } // move tab to a new window frameOpts = frameOpts.set('index', 0) appActions.newWindow(frameOpts, browserOpts) @@ -1049,13 +1075,25 @@ const api = { } // handle tab has made it to the new window tab.once('did-attach', () => { + if (shouldDebugTabEvents) { + console.log(`Tab attached to a new window, so setting the desired index`) + } // put the tab in the desired index position const index = frameOpts.get('index') if (index !== undefined) { api.setTabIndex(tabId, frameOpts.get('index')) } }) - }) + } + // detach + if (tabValue.get('discarded')) { + // if the tab is discarded, detaching will never complete + // callback / 'did-detach' will never fire + tab.detach() + attachToNewWindow() + } else { + tab.detach(attachToNewWindow) + } } }, diff --git a/app/browser/webContentsCache.js b/app/browser/webContentsCache.js index e483ffb3bf6..0c8c580b85b 100644 --- a/app/browser/webContentsCache.js +++ b/app/browser/webContentsCache.js @@ -31,11 +31,23 @@ const forgetOpenerForTabId = (tabId) => { } } +const tabIdChanged = (oldTabId, newTabId) => { + // any tabs referencing the old contents Id as the opener, + // should now reference the new contents Id + for (const tabId in currentWebContents) { + const tabData = currentWebContents[tabId] + if (tabData && tabData.openerTabId != null && tabData.openerTabId === oldTabId) { + tabData.openerTabId = newTabId + } + } +} + module.exports = { cleanupWebContents, getWebContents, getOpenerTabId, forgetOpenerForTabId, updateWebContents, + tabIdChanged, currentWebContents } diff --git a/app/renderer/components/frame/frame.js b/app/renderer/components/frame/frame.js index 603a717171f..900e27b48d4 100644 --- a/app/renderer/components/frame/frame.js +++ b/app/renderer/components/frame/frame.js @@ -437,8 +437,9 @@ class Frame extends React.Component { if (this.frame.isEmpty()) { return } - - windowActions.frameTabIdChanged(this.frame, this.props.tabId, e.tabID) + if (this.props.tabId !== e.tabID) { + appActions.tabReplaced(this.props.tabId, e.tabID) + } }, { passive: true }) this.webview.addEventListener('guest-ready', (e) => { if (this.frame.isEmpty()) { diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 1cde5b5e522..c47f779e51b 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -302,6 +302,15 @@ const appActions = { }) }, + tabReplaced: function (oldTabId, newTabId) { + console.log('tab replaced', {oldTabId, newTabId}) + dispatch({ + actionType: appConstants.APP_TAB_REPLACED, + oldTabId, + newTabId + }) + }, + /** * Dispatches a message to the store to set a new frame as the active frame. * diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 3d303216de0..b5cfe4ec3dd 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -15,6 +15,7 @@ const appConstants = { APP_TAB_STRIP_EMPTY: _, APP_TAB_CLOSED: _, APP_TAB_UPDATED: _, + APP_TAB_REPLACED: _, APP_ADD_HISTORY_SITE: _, APP_SET_STATE: _, APP_REMOVE_HISTORY_SITE: _, diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index c539d1a33f9..e6b34c36757 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -184,7 +184,12 @@ const frameTabIdChanged = (state, action) => { let newFrameProps = new Immutable.Map() newFrameProps = newFrameProps.set('tabId', newTabId) - const index = frameStateUtil.getFrameIndex(state, action.getIn(['frameProps', 'key'])) + const frame = frameStateUtil.getFrameByTabId(state, oldTabId) + if (!frame) { + console.error(`Could not find frame with tabId ${oldTabId} in order to replace with new tabId ${newTabId}`) + return state + } + const index = frameStateUtil.getFrameIndex(state, frame.get('key')) state = state.mergeIn(['frames', index], newFrameProps) state = frameStateUtil.deleteTabInternalIndex(state, oldTabId) state = frameStateUtil.updateFramesInternalIndex(state, index) @@ -263,7 +268,7 @@ const doAction = (action) => { } // We should not emit here because the Window already know about the change on startup. return - case windowConstants.WINDOW_FRAME_TAB_ID_CHANGED: + case appConstants.APP_TAB_REPLACED: windowState = frameTabIdChanged(windowState, action) break case windowConstants.WINDOW_FRAME_GUEST_INSTANCE_ID_CHANGED: