From a24749f41d488a3e4da6045b1d0a54830f2c9fce Mon Sep 17 00:00:00 2001 From: NejcZdovc Date: Thu, 4 May 2017 10:26:04 +0200 Subject: [PATCH] Merges tabs into frames object in windowStore Resolves #8689 Auditors: @bsclifton @bbondy Test Plan: - tests should be green --- .../components/tabs/content/audioTabIcon.js | 9 ++- .../components/tabs/content/closeTabIcon.js | 2 +- .../components/tabs/content/favIcon.js | 8 +- .../components/tabs/content/newSessionIcon.js | 2 +- .../components/tabs/content/privateIcon.js | 2 +- .../components/tabs/content/tabTitle.js | 4 +- app/renderer/components/tabs/pinnedTabs.js | 12 +-- app/renderer/components/tabs/tab.js | 78 +++++++++---------- app/renderer/components/tabs/tabs.js | 12 +-- app/renderer/components/tabs/tabsToolbar.js | 4 +- app/renderer/lib/tabUtil.js | 12 +-- app/renderer/reducers/frameReducer.js | 1 - app/renderer/reducers/urlBarReducer.js | 14 +--- app/sessionStore.js | 8 -- docs/state.md | 19 +---- js/components/main.js | 2 +- js/state/frameStateUtil.js | 54 +------------ js/stores/windowStore.js | 33 +------- test/about/preferencesTest.js | 6 +- .../tabs/content/audioTabIconTest.js | 10 +-- .../tabs/content/closeTabIconTest.js | 28 +++---- .../components/tabs/content/favIconTest.js | 8 +- .../tabs/content/newSessionIconTest.js | 34 ++++---- .../tabs/content/privateIconTest.js | 28 +++---- .../components/tabs/content/tabTitleTest.js | 20 ++--- test/unit/js/stores/windowStoreTest.js | 6 -- test/unit/state/frameStateUtilTest.js | 41 +++------- 27 files changed, 160 insertions(+), 297 deletions(-) diff --git a/app/renderer/components/tabs/content/audioTabIcon.js b/app/renderer/components/tabs/content/audioTabIcon.js index 644562adb6e..bde0765c55d 100644 --- a/app/renderer/components/tabs/content/audioTabIcon.js +++ b/app/renderer/components/tabs/content/audioTabIcon.js @@ -15,16 +15,16 @@ const tabStyles = require('../../styles/tab') class AudioTabIcon extends ImmutableComponent { get pageCanPlayAudio () { - return !!this.props.tab.get('audioPlaybackActive') + return !!this.props.frame.get('audioPlaybackActive') } get shouldShowAudioIcon () { // We switch to blue top bar for all breakpoints but default - return this.props.tab.get('breakpoint') === 'default' + return this.props.frame.get('breakpoint') === 'default' } get mutedState () { - return this.pageCanPlayAudio && !!this.props.tab.get('audioMuted') + return this.pageCanPlayAudio && !!this.props.frame.get('audioMuted') } get audioIcon () { @@ -37,7 +37,8 @@ class AudioTabIcon extends ImmutableComponent { return this.pageCanPlayAudio && this.shouldShowAudioIcon ? + symbol={this.audioIcon} + onClick={this.props.onClick} /> : null } } diff --git a/app/renderer/components/tabs/content/closeTabIcon.js b/app/renderer/components/tabs/content/closeTabIcon.js index 5875492a977..33d3afc3ce7 100644 --- a/app/renderer/components/tabs/content/closeTabIcon.js +++ b/app/renderer/components/tabs/content/closeTabIcon.js @@ -19,7 +19,7 @@ const closeTabSvg = require('../../../../extensions/brave/img/tabs/close_btn_nor class CloseTabIcon extends ImmutableComponent { get isPinned () { - return !!this.props.tab.get('pinnedLocation') + return !!this.props.frame.get('pinnedLocation') } render () { diff --git a/app/renderer/components/tabs/content/favIcon.js b/app/renderer/components/tabs/content/favIcon.js index f996c372a84..b04f886c446 100644 --- a/app/renderer/components/tabs/content/favIcon.js +++ b/app/renderer/components/tabs/content/favIcon.js @@ -20,7 +20,7 @@ const loadingIconSvg = require('../../../../extensions/brave/img/tabs/loading.sv class Favicon extends ImmutableComponent { get favicon () { - return !this.props.isLoading && this.props.tab.get('icon') + return !this.props.isLoading && this.props.frame.get('icon') } get defaultIcon () { @@ -30,12 +30,12 @@ class Favicon extends ImmutableComponent { } get narrowView () { - return this.props.tab.get('breakpoint') === 'smallest' + return this.props.frame.get('breakpoint') === 'smallest' } get shouldHideFavicon () { return (hasBreakpoint(this.props, 'extraSmall') && this.props.isActive) || - this.props.tab.get('location') === 'about:newtab' + this.props.frame.get('location') === 'about:newtab' } render () { @@ -57,7 +57,7 @@ class Favicon extends ImmutableComponent { className={css( tabStyles.icon, this.favicon && iconStyles.favicon, - !this.props.tab.get('pinnedLocation') && this.narrowView && styles.faviconNarrowView + !this.props.frame.get('pinnedLocation') && this.narrowView && styles.faviconNarrowView )} symbol={ (this.props.isLoading && css(styles.loadingIcon, iconStyles.loadingIconColor)) || diff --git a/app/renderer/components/tabs/content/newSessionIcon.js b/app/renderer/components/tabs/content/newSessionIcon.js index 90142337dfb..3ecd100fca4 100644 --- a/app/renderer/components/tabs/content/newSessionIcon.js +++ b/app/renderer/components/tabs/content/newSessionIcon.js @@ -21,7 +21,7 @@ const newSessionSvg = require('../../../../extensions/brave/img/tabs/new_session class NewSessionIcon extends ImmutableComponent { get partitionNumber () { - let partition = this.props.tab.get('partitionNumber') + let partition = this.props.frame.get('partitionNumber') // Persistent partitions opened by `target="_blank"` will have // *partition-* string first, which causes bad UI. We don't need it for tabs if (typeof partition === 'string') { diff --git a/app/renderer/components/tabs/content/privateIcon.js b/app/renderer/components/tabs/content/privateIcon.js index 94e390f3123..74ace2624a1 100644 --- a/app/renderer/components/tabs/content/privateIcon.js +++ b/app/renderer/components/tabs/content/privateIcon.js @@ -25,7 +25,7 @@ class PrivateIcon extends ImmutableComponent { } }) - return this.props.tab.get('isPrivate') && hasVisibleSecondaryIcon(this.props) + return this.props.frame.get('isPrivate') && hasVisibleSecondaryIcon(this.props) ? diff --git a/app/renderer/components/tabs/content/tabTitle.js b/app/renderer/components/tabs/content/tabTitle.js index 4af91b649bb..8fdac918cfa 100644 --- a/app/renderer/components/tabs/content/tabTitle.js +++ b/app/renderer/components/tabs/content/tabTitle.js @@ -18,11 +18,11 @@ const globalStyles = require('../../styles/global') class TabTitle extends ImmutableComponent { get isActiveOrHasSecondaryIcon () { return this.props.isActive || - (!!this.props.tab.get('isPrivate') || !!this.props.tab.get('partitionNumber')) + (!!this.props.frame.get('isPrivate') || !!this.props.frame.get('partitionNumber')) } get isPinned () { - return !!this.props.tab.get('pinnedLocation') + return !!this.props.frame.get('pinnedLocation') } get shouldHideTitle () { diff --git a/app/renderer/components/tabs/pinnedTabs.js b/app/renderer/components/tabs/pinnedTabs.js index 13a2a0cfd6b..cdc4d8e019e 100644 --- a/app/renderer/components/tabs/pinnedTabs.js +++ b/app/renderer/components/tabs/pinnedTabs.js @@ -45,10 +45,10 @@ class PinnedTabs extends ImmutableComponent { // will cause the onDragEnd to never run setTimeout(() => { const key = sourceDragData.get('key') - let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.tab.get('frameKey') !== key), clientX).selectedRef + let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.frame.get('key') !== key), clientX).selectedRef if (droppedOnTab) { const isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOnTab), clientX) - const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.tab.get('frameKey')) + const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.frame.get('key')) windowActions.moveTab(sourceDragData, droppedOnFrameProps, isLeftSide) if (!sourceDragData.get('pinnedLocation')) { appActions.tabPinned(sourceDragData.get('tabId'), true) @@ -73,14 +73,14 @@ class PinnedTabs extends ImmutableComponent { onDrop={this.onDrop}> { this.props.pinnedTabs - .map((tab) => + .map((frame) => this.tabRefs.push(node)} dragData={this.props.dragData} - tab={tab} - key={'tab-' + tab.get('frameKey')} + frame={frame} + key={'tab-' + frame.get('key')} paintTabs={this.props.paintTabs} previewTabs={this.props.previewTabs} - isActive={this.props.activeFrameKey === tab.get('frameKey')} + isActive={this.props.activeFrameKey === frame.get('key')} partOfFullPageSet={this.props.partOfFullPageSet} />) } diff --git a/app/renderer/components/tabs/tab.js b/app/renderer/components/tabs/tab.js index 3371cfdb962..1dadf1adbeb 100644 --- a/app/renderer/components/tabs/tab.js +++ b/app/renderer/components/tabs/tab.js @@ -51,16 +51,16 @@ class Tab extends ImmutableComponent { this.onUpdateTabSize = this.onUpdateTabSize.bind(this) } get frame () { - return windowStore.getFrame(this.props.tab.get('frameKey')) + return windowStore.getFrame(this.props.frame.get('key')) } get isPinned () { - return !!this.props.tab.get('pinnedLocation') + return !!this.props.frame.get('pinnedLocation') } get draggingOverData () { const draggingOverData = this.props.dragData && this.props.dragData.get('dragOverData') if (!draggingOverData || - draggingOverData.get('draggingOverKey') !== this.props.tab.get('frameKey') || + draggingOverData.get('draggingOverKey') !== this.props.frame.get('key') || draggingOverData.get('draggingOverWindowId') !== getCurrentWindowId()) { return } @@ -83,7 +83,7 @@ class Tab extends ImmutableComponent { get isDragging () { const sourceDragData = dnd.getInterBraveDragData() return sourceDragData && - sourceDragData.get('key') === this.props.tab.get('frameKey') && + sourceDragData.get('key') === this.props.frame.get('key') && sourceDragData.get('draggingOverWindowId') === getCurrentWindowId() } @@ -115,17 +115,17 @@ class Tab extends ImmutableComponent { // until we know what we're loading. We should probably do this for // all about: pages that we already know the title for so we don't have // to wait for the title to be parsed. - if (this.props.tab.get('location') === 'about:blank') { + if (this.props.frame.get('location') === 'about:blank') { return locale.translation('aboutBlankTitle') - } else if (this.props.tab.get('location') === 'about:newtab') { + } else if (this.props.frame.get('location') === 'about:newtab') { return locale.translation('newTab') } // YouTube tries to change the title to add a play icon when // there is audio. Since we have our own audio indicator we get // rid of it. - return (this.props.tab.get('title') || - this.props.tab.get('location') || + return (this.props.frame.get('title') || + this.props.frame.get('location') || '').replace('▶ ', '') } @@ -138,7 +138,7 @@ class Tab extends ImmutableComponent { } onDragOver (e) { - dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.tab.get('frameKey'), this.draggingOverData, e) + dnd.onDragOver(dragTypes.TAB, this.tabNode.getBoundingClientRect(), this.props.frame.get('key'), this.draggingOverData, e) } onTabClosedWithMouse (event) { @@ -157,10 +157,10 @@ class Tab extends ImmutableComponent { get loading () { return this.frame && - (this.props.tab.get('loading') || - this.props.tab.get('location') === 'about:blank') && - (!this.props.tab.get('provisionalLocation') || - !this.props.tab.get('provisionalLocation').startsWith('chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/')) + (this.props.frame.get('loading') || + this.props.frame.get('location') === 'about:blank') && + (!this.props.frame.get('provisionalLocation') || + !this.props.frame.get('provisionalLocation').startsWith('chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd/')) } onMouseLeave () { @@ -201,7 +201,7 @@ class Tab extends ImmutableComponent { get themeColor () { return this.props.paintTabs && - (this.props.tab.get('themeColor') || this.props.tab.get('computedThemeColor')) + (this.props.frame.get('themeColor') || this.props.frame.get('computedThemeColor')) } get tabSize () { @@ -212,21 +212,21 @@ class Tab extends ImmutableComponent { get mediumView () { const sizes = ['large', 'largeMedium'] - return sizes.includes(this.props.tab.get('breakpoint')) + return sizes.includes(this.props.frame.get('breakpoint')) } get narrowView () { const sizes = ['medium', 'mediumSmall', 'small', 'extraSmall', 'smallest'] - return sizes.includes(this.props.tab.get('breakpoint')) + return sizes.includes(this.props.frame.get('breakpoint')) } get narrowestView () { const sizes = ['extraSmall', 'smallest'] - return sizes.includes(this.props.tab.get('breakpoint')) + return sizes.includes(this.props.frame.get('breakpoint')) } get canPlayAudio () { - return this.props.tab.get('audioPlaybackActive') || this.props.tab.get('audioMuted') + return this.props.frame.get('audioPlaybackActive') || this.props.frame.get('audioMuted') } onUpdateTabSize () { @@ -263,10 +263,10 @@ class Tab extends ImmutableComponent { render () { const notificationBarActive = !!this.props.notificationBarActive && - this.props.notificationBarActive.includes(UrlUtil.getUrlOrigin(this.props.tab.get('location'))) + this.props.notificationBarActive.includes(UrlUtil.getUrlOrigin(this.props.frame.get('location'))) const playIndicatorBreakpoint = this.mediumView || this.narrowView // we don't want themeColor if tab is private - const perPageStyles = !this.props.tab.get('isPrivate') && StyleSheet.create({ + const perPageStyles = !this.props.frame.get('isPrivate') && StyleSheet.create({ themeColor: { color: this.themeColor ? getTextColorForBackground(this.themeColor) : 'inherit', background: this.themeColor ? this.themeColor : 'inherit', @@ -302,20 +302,20 @@ class Tab extends ImmutableComponent { playIndicatorBreakpoint && this.canPlayAudio && styles.narrowViewPlayIndicator, this.props.isActive && this.themeColor && perPageStyles.themeColor, // Private color should override themeColor - this.props.tab.get('isPrivate') && styles.private, - this.props.isActive && this.props.tab.get('isPrivate') && styles.activePrivateTab, + this.props.frame.get('isPrivate') && styles.private, + this.props.isActive && this.props.frame.get('isPrivate') && styles.activePrivateTab, !this.isPinned && this.narrowView && styles.tabNarrowView, !this.isPinned && this.narrowestView && styles.tabNarrowestView, - !this.isPinned && this.props.tab.get('breakpoint') === 'smallest' && styles.tabMinAllowedSize + !this.isPinned && this.props.frame.get('breakpoint') === 'smallest' && styles.tabMinAllowedSize )} data-test-active-tab={this.props.isActive} data-test-pinned-tab={this.isPinned} - data-test-private-tab={this.props.tab.get('isPrivate')} + data-test-private-tab={this.props.frame.get('isPrivate')} data-test-id='tab' - data-frame-key={this.props.tab.get('frameKey')} + data-frame-key={this.props.frame.get('key')} ref={(node) => { this.tabNode = node }} draggable - title={this.props.tab.get('title')} + title={this.props.frame.get('title')} onDragStart={this.onDragStart.bind(this)} onDragEnd={this.onDragEnd.bind(this)} onDragOver={this.onDragOver.bind(this)} @@ -324,40 +324,40 @@ class Tab extends ImmutableComponent {
@@ -375,11 +375,11 @@ const paymentsEnabled = () => { windowStore.addChangeListener(() => { if (paymentsEnabled()) { const windowState = windowStore.getState() - const tabs = windowState && windowState.get('tabs') - if (tabs) { + const frames = windowState && windowState.get('frames') + if (frames) { try { - const presentP = tabs.some((tab) => { - return tab.get('location') === 'about:preferences#payments' + const presentP = frames.some((frame) => { + return frame.get('location') === 'about:preferences#payments' }) ipc.send(messages.LEDGER_PAYMENTS_PRESENT, presentP) } catch (ex) { } diff --git a/app/renderer/components/tabs/tabs.js b/app/renderer/components/tabs/tabs.js index f74829057fe..4c47e35bb8a 100644 --- a/app/renderer/components/tabs/tabs.js +++ b/app/renderer/components/tabs/tabs.js @@ -79,10 +79,10 @@ class Tabs extends ImmutableComponent { // will cause the onDragEnd to never run setTimeout(() => { const key = sourceDragData.get('key') - let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.tab.get('frameKey') !== key), clientX).selectedRef + let droppedOnTab = dnd.closestFromXOffset(this.tabRefs.filter((node) => node && node.props.frame.get('key') !== key), clientX).selectedRef if (droppedOnTab) { const isLeftSide = dnd.isLeftSide(ReactDOM.findDOMNode(droppedOnTab), clientX) - const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.tab.get('frameKey')) + const droppedOnFrameProps = windowStore.getFrame(droppedOnTab.props.frame.get('key')) // If this is a different window ID than where the drag started, then // the tear off will be done by tab.js @@ -149,14 +149,14 @@ class Tabs extends ImmutableComponent { })()} { this.props.currentTabs - .map((tab) => + .map((frame) => this.tabRefs.push(node)} dragData={this.props.dragData} - tab={tab} - key={'tab-' + tab.get('frameKey')} + frame={frame} + key={'tab-' + frame.get('key')} paintTabs={this.props.paintTabs} previewTabs={this.props.previewTabs} - isActive={this.props.activeFrameKey === tab.get('frameKey')} + isActive={this.props.activeFrameKey === frame.get('key')} onTabClosedWithMouse={this.onTabClosedWithMouse} tabWidth={this.props.fixTabWidth} hasTabInFullScreen={this.props.hasTabInFullScreen} diff --git a/app/renderer/components/tabs/tabsToolbar.js b/app/renderer/components/tabs/tabsToolbar.js index 476b71daaee..81a1a4378bb 100644 --- a/app/renderer/components/tabs/tabsToolbar.js +++ b/app/renderer/components/tabs/tabsToolbar.js @@ -34,8 +34,8 @@ class TabsToolbar extends ImmutableComponent { const index = this.props.previewTabPageIndex !== undefined ? this.props.previewTabPageIndex : this.props.tabPageIndex const startingFrameIndex = index * this.props.tabsPerTabPage - const pinnedTabs = this.props.tabs.filter((tab) => tab.get('pinnedLocation')) - const unpinnedTabs = this.props.tabs.filter((tab) => !tab.get('pinnedLocation')) + const pinnedTabs = this.props.frames.filter((tab) => tab.get('pinnedLocation')) + const unpinnedTabs = this.props.frames.filter((tab) => !tab.get('pinnedLocation')) const currentTabs = unpinnedTabs .slice(startingFrameIndex, startingFrameIndex + this.props.tabsPerTabPage) return
{ arr = Array.isArray(arr) ? arr : [arr] - return arr.includes(props.tab.get('breakpoint')) + return arr.includes(props.frame.get('breakpoint')) } /** @@ -47,7 +47,7 @@ module.exports.hasBreakpoint = (props, arr) => { * @returns {Boolean} Whether or not the tab has a relative closeTab icon */ module.exports.hasRelativeCloseIcon = (props) => { - return props.tab.get('hoverState') && + return props.frame.get('hoverState') && module.exports.hasBreakpoint(props, ['default', 'large']) } @@ -84,13 +84,13 @@ module.exports.hasFixedCloseIcon = (props) => { /** * Gets the icon color based on tab's background - * @param {Object} props - Object that hosts the tab props + * @param {Object} props - Object that hosts the frame props * @returns {String} Contrasting color to use based on tab's color */ module.exports.getTabIconColor = (props) => { - const themeColor = props.tab.get('themeColor') || props.tab.get('computedThemeColor') - const activeNonPrivateTab = !props.tab.get('isPrivate') && props.isActive - const isPrivateTab = props.tab.get('isPrivate') && (props.isActive || props.tab.get('hoverState')) + const themeColor = props.frame.get('themeColor') || props.frame.get('computedThemeColor') + const activeNonPrivateTab = !props.frame.get('isPrivate') && props.isActive + const isPrivateTab = props.frame.get('isPrivate') && (props.isActive || props.frame.get('hoverState')) const defaultColor = isPrivateTab ? styles.color.white100 : styles.color.black100 return activeNonPrivateTab && props.paintTabs && !!themeColor diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js index 9b53f117978..cbca8268fe6 100644 --- a/app/renderer/reducers/frameReducer.js +++ b/app/renderer/reducers/frameReducer.js @@ -33,7 +33,6 @@ const closeFrame = (state, action) => { const activeFrameKey = frameStateUtil.getActiveFrame(state).get('key') state = state.merge(frameStateUtil.removeFrame( state.get('frames'), - state.get('tabs'), state.get('closedFrames'), frameProps.set('closedAtIndex', index), activeFrameKey, diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index 2ae163afafb..1b08ebcae2b 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -7,7 +7,7 @@ const windowConstants = require('../../../js/constants/windowConstants') const {aboutUrls, isNavigatableAboutPage, isSourceAboutUrl, isUrl, getSourceAboutUrl, getSourceMagnetUrl} = require('../../../js/lib/appUrlUtil') const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/lib/urlutil') -const {getFrameByKey, getFrameKeyByTabId, activeFrameStatePath, frameStatePath, frameStatePathForFrame, getActiveFrame, tabStatePath, getFrameByTabId} = require('../../../js/state/frameStateUtil') +const {getFrameByKey, getFrameKeyByTabId, activeFrameStatePath, frameStatePath, frameStatePathForFrame, getActiveFrame, getFrameByTabId} = require('../../../js/state/frameStateUtil') const getSetting = require('../../../js/settings').getSetting const {isBookmark, isDefaultEntry, isHistoryEntry} = require('../../../js/state/siteUtil') const fetchSearchSuggestions = require('../fetchSearchSuggestions') @@ -400,9 +400,6 @@ const urlBarReducer = (state, action) => { state = state.mergeIn(frameStatePath(state, key), { location: action.location }) - state = state.mergeIn(tabStatePath(state, key), { - location: action.location - }) if (!action.isNavigatedInPage) { state = state.mergeIn(frameStatePath(state, key), { adblock: {}, @@ -417,15 +414,6 @@ const urlBarReducer = (state, action) => { trackingProtection: {}, fingerprintingProtection: {} }) - // TODO: This should be moved into a tabs reducer - state = state.mergeIn(tabStatePath(state, key), { - audioPlaybackActive: false, - themeColor: undefined, - location: action.location, - computedThemeColor: undefined, - icon: undefined, - title: '' - }) } // Update nav bar unless when spawning a new tab. The user might have diff --git a/app/sessionStore.js b/app/sessionStore.js index c17e3cdcea5..eac0e207d40 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -20,7 +20,6 @@ const locale = require('./locale') const UpdateStatus = require('../js/constants/updateStatus') const settings = require('../js/constants/settings') const downloadStates = require('../js/constants/downloadStates') -const {tabFromFrame} = require('../js/state/frameStateUtil') const siteUtil = require('../js/state/siteUtil') const { topSites, pinnedTopSites } = require('../js/data/newTabData') const sessionStorageVersion = 1 @@ -78,11 +77,6 @@ module.exports.saveAppState = (payload, isShutdown) => { payload.perWindowState.forEach((wndPayload) => { wndPayload.frames = wndPayload.frames.filter((frame) => !frame.isPrivate) }) - // tabs will be auto-reset to what the frame is in cleanAppData but just in - // case clean fails we don't want to save private tabs. - payload.perWindowState.forEach((wndPayload) => { - wndPayload.tabs = wndPayload.tabs.filter((tab) => !tab.isPrivate) - }) } else { delete payload.perWindowState } @@ -238,8 +232,6 @@ module.exports.cleanPerWindowData = (perWindowData, isShutdown) => { .filter((frame) => !frame.pinnedLocation) perWindowData.frames.forEach(cleanFrame) } - // Always recalculate tab data from frame data - perWindowData.tabs = perWindowData.frames.map((frame) => tabFromFrame(frame)) } /** diff --git a/docs/state.md b/docs/state.md index 9a69bacb0cb..cd3e98b9b69 100644 --- a/docs/state.md +++ b/docs/state.md @@ -414,6 +414,7 @@ WindowStore audioMuted: boolean, // frame is muted audioPlaybackActive: boolean, // frame is playing audio basicAuthDetail: object, + breakpoint: string, // breakpoint name for current tab size, specified in app/renderer/components/styles/tab.js closedAtIndex: number, // index the frame was last closed at, cleared unless the frame is inside of closedFrames computedThemeColor: string, // CSS computed theme color from the favicon endtLoadTime: datetime, @@ -433,6 +434,7 @@ WindowStore history: array, // navigation history hrefPreview: string, // show hovered link preview httpsEverywhere: Object>, // map of XML rulesets name to redirected resources + hoverState: boolean, // wheter or not tab is being hovered icon: string, // favicon url isFullScreen: boolean, // true if the frame should be shown as full screen isPrivate: boolean, // private browsing tab @@ -614,23 +616,6 @@ WindowStore autocompleteURL: string, // ditto re: {searchTerms} searchURL: string // with replacement var in string: {searchTerms} }, - tabs: [{ - audioMuted: boolean, // frame is muted - audioPlaybackActive: boolean, // frame is playing audio - breakpoint: string, // breakpoint name for current tab size, specified in app/renderer/components/styles/tab.js - computedThemeColor: string, // CSS computed theme color from the favicon - frameKey: number, - hoverState: boolean, // wheter or not tab is being hovered - icon: string, // favicon url - isPrivate: boolean, // private browsing tab - loading: boolean, - location: string, // the currently navigated location - partitionNumber: number, // the session partition to use - pinnedLocation: string, // Indicates if a frame is pinned and its pin location - provisionalLocation: string, - themeColor: string, // CSS compatible color string - title: string // page title - }], ui: { bookmarksToolbar: { selectedFolderId: number // folderId from the siteDetail of the currently expanded folder diff --git a/js/components/main.js b/js/components/main.js index dc773e3d2b1..0a936524169 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -861,7 +861,7 @@ class Main extends ImmutableComponent { tabPageIndex={this.props.windowState.getIn(['ui', 'tabs', 'tabPageIndex'])} previewTabPageIndex={this.props.windowState.getIn(['ui', 'tabs', 'previewTabPageIndex'])} fixTabWidth={this.props.windowState.getIn(['ui', 'tabs', 'fixTabWidth'])} - tabs={this.props.windowState.get('tabs')} + frames={this.props.windowState.get('frames')} sites={appStateSites} key='tab-bar' activeFrameKey={(activeFrame && activeFrame.get('key')) || undefined} diff --git a/js/state/frameStateUtil.js b/js/state/frameStateUtil.js index 60aa8e8e355..a934b183c2b 100644 --- a/js/state/frameStateUtil.js +++ b/js/state/frameStateUtil.js @@ -382,44 +382,6 @@ function getPartitionFromNumber (partitionNumber, incognito) { return `persist:partition-${partitionNumber}` } -/** - * Returns an object in the same format that was passed to it (ImmutableJS/POD) - * for the subset of frame data that is used for tabs. - */ -const tabFromFrame = (frame) => { - return frame.toJS - ? Immutable.fromJS({ - themeColor: frame.get('themeColor'), - computedThemeColor: frame.get('computedThemeColor'), - icon: frame.get('icon'), - audioPlaybackActive: frame.get('audioPlaybackActive'), - audioMuted: frame.get('audioMuted'), - title: frame.get('title'), - isPrivate: frame.get('isPrivate'), - partitionNumber: frame.get('partitionNumber'), - frameKey: frame.get('key'), - loading: isFrameLoading(frame), - provisionalLocation: frame.get('provisionalLocation'), - pinnedLocation: frame.get('pinnedLocation'), - location: frame.get('location') - }) - : { - themeColor: frame.themeColor, - computedThemeColor: frame.computedThemeColor, - icon: frame.icon, - audioPlaybackActive: frame.audioPlaybackActive, - audioMuted: frame.audioMuted, - title: frame.title, - isPrivate: frame.isPrivate, - partitionNumber: frame.partitionNumber, - frameKey: frame.key, - loading: frame.loading, - provisionalLocation: frame.provisionalLocation, - pinnedLocation: frame.pinnedLocation, - location: frame.location - } -} - const frameOptsFromFrame = (frame) => { return frame .delete('key') @@ -433,7 +395,7 @@ const frameOptsFromFrame = (frame) => { * Adds a frame specified by frameOpts and newKey and sets the activeFrameKey * @return Immutable top level application state ready to merge back in */ -function addFrame (windowState, tabs, frameOpts, newKey, partitionNumber, activeFrameKey, insertionIndex) { +function addFrame (windowState, frameOpts, newKey, partitionNumber, activeFrameKey, insertionIndex) { const frames = windowState.get('frames') const url = frameOpts.location || config.defaultUrl @@ -499,7 +461,6 @@ function addFrame (windowState, tabs, frameOpts, newKey, partitionNumber, active }, frameOpts)) return { - tabs: tabs.splice(insertionIndex, 0, tabFromFrame(frame)), frames: frames.splice(insertionIndex, 0, frame), activeFrameKey } @@ -509,7 +470,7 @@ function addFrame (windowState, tabs, frameOpts, newKey, partitionNumber, active * Removes a frame specified by frameProps * @return Immutable top level application state ready to merge back in */ -function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex, closeAction) { +function removeFrame (frames, closedFrames, frameProps, activeFrameKey, framePropsIndex, closeAction) { function getNewActiveFrame (activeFrameIndex) { // Go to the next frame if it exists. let index = activeFrameIndex @@ -550,7 +511,6 @@ function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey, fr } const newFrames = frames.splice(framePropsIndex, 1) - const newTabs = tabs.splice(framePropsIndex, 1) let newActiveFrameKey = activeFrameKey // If the frame being removed IS ACTIVE @@ -581,7 +541,6 @@ function removeFrame (frames, tabs, closedFrames, frameProps, activeFrameKey, fr previewFrameKey: null, activeFrameKey: newActiveFrameKey, closedFrames, - tabs: newTabs, frames: newFrames } } @@ -633,12 +592,6 @@ const activeFrameStatePath = (windowState) => frameStatePath(windowState, window const frameStatePathForFrame = (windowState, frameProps) => ['frames', getFramePropsIndex(windowState.get('frames'), frameProps)] -const tabStatePath = (windowState, frameKey) => - ['tabs', findIndexForFrameKey(windowState.get('frames'), frameKey)] - -const tabStatePathForFrame = (windowState, frameProps) => - ['tabs', getFramePropsIndex(windowState.get('frames'), frameProps)] - module.exports = { query, find, @@ -686,15 +639,12 @@ module.exports = { getPartitionFromNumber, addFrame, removeFrame, - tabFromFrame, frameOptsFromFrame, getFrameKeyByTabId, getFrameTabPageIndex, frameStatePath, activeFrameStatePath, frameStatePathForFrame, - tabStatePath, - tabStatePathForFrame, getLastCommittedURL, getFrameByLastAccessedTime, onFindBarHide, diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index a981a90beed..7bf00fcc1f4 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -12,14 +12,13 @@ const config = require('../constants/config') const settings = require('../constants/settings') const Immutable = require('immutable') const frameStateUtil = require('../state/frameStateUtil') -const {activeFrameStatePath, frameStatePathForFrame, tabStatePathForFrame} = frameStateUtil +const {activeFrameStatePath, frameStatePathForFrame} = frameStateUtil const ipc = require('electron').ipcRenderer const messages = require('../constants/messages') const debounce = require('../lib/debounce') const getSetting = require('../settings').getSetting const UrlUtil = require('../lib/urlutil') const {getCurrentWindowId, isFocused} = require('../../app/renderer/currentWindow') -const {tabFromFrame} = require('../state/frameStateUtil') const {l10nErrorText} = require('../../app/common/lib/httpUtil') const { makeImmutable } = require('../../app/common/state/immutableUtil') const {aboutUrls, getTargetAboutUrl, newFrameUrl} = require('../lib/appUrlUtil') @@ -35,7 +34,6 @@ let previousTabs = new Immutable.List() let windowState = Immutable.fromJS({ activeFrameKey: null, frames: [], - tabs: [], closedFrames: [], ui: { tabs: { @@ -81,10 +79,6 @@ class WindowStore extends EventEmitter { return frameStateUtil.getFrameByKey(windowState, key) } - getFrameCount () { - return this.getFrames().size - } - emitChanges () { if (lastEmittedState !== windowState) { lastEmittedState = windowState @@ -189,7 +183,7 @@ const newFrame = (frameOpts, openInForeground, insertionIndex, nextKey) => { windowState = windowState.merge( frameStateUtil.addFrame( - windowState, windowState.get('tabs'), frameOpts, + windowState, frameOpts, nextKey, frameOpts.partitionNumber, openInForeground || typeof windowState.get('activeFrameKey') !== 'number' ? nextKey : windowState.get('activeFrameKey'), insertionIndex)) if (openInForeground) { @@ -241,7 +235,6 @@ const tabDataChanged = (state, action) => { const frameProps = frameStateUtil.getFrameByTabId(state, tab.get('tabId')) if (frameProps) { state = state.mergeIn(['frames', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(state), frameProps)], newProps) - state = state.mergeIn(['tabs', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(state), frameProps)], newProps) if (tab.get('active') === true && tab.get('windowId') === getCurrentWindowId()) { state = state.merge({ activeFrameKey: frameProps.get('key'), @@ -321,9 +314,7 @@ const doAction = (action) => { case windowConstants.WINDOW_SET_FINDBAR_SHOWN: const frameIndex = frameStateUtil.findIndexForFrameKey(windowState.get('frames'), action.frameKey) windowState = windowState.mergeIn(['frames', frameIndex], { - findbarShown: action.shown - }) - windowState = windowState.mergeIn(['frames', frameIndex], { + findbarShown: action.shown, findbarSelected: action.shown }) break @@ -351,10 +342,6 @@ const doAction = (action) => { startLoadTime: new Date().getTime(), endLoadTime: null }) - windowState = windowState.mergeIn(statePath('tabs'), { - loading: true, - provisionalLocation: action.location - }) // For about:newtab we want to have the urlbar focused, not the new frame. // Otherwise we want to focus the new tab when it is a new frame in the foreground. if (action.location !== getTargetAboutUrl('about:newtab')) { @@ -368,9 +355,6 @@ const doAction = (action) => { endLoadTime: new Date().getTime(), history: addToHistory(action.frameProps) }) - windowState = windowState.mergeIn(['tabs', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps)], { - loading: false - }) break case windowConstants.WINDOW_UNDO_CLOSED_FRAME: { @@ -414,24 +398,19 @@ const doAction = (action) => { break case windowConstants.WINDOW_SET_TAB_BREAKPOINT: windowState = windowState.setIn(['frames', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'breakpoint'], action.breakpoint) - windowState = windowState.setIn(['tabs', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'breakpoint'], action.breakpoint) break case windowConstants.WINDOW_SET_TAB_HOVER_STATE: windowState = windowState.setIn(['frames', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'hoverState'], action.hoverState) - windowState = windowState.setIn(['tabs', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'hoverState'], action.hoverState) break case windowConstants.WINDOW_TAB_MOVE: const sourceFramePropsIndex = frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.sourceFrameProps) let newIndex = frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.destinationFrameProps) + (action.prepend ? 0 : 1) let frames = frameStateUtil.getFrames(windowState).splice(sourceFramePropsIndex, 1) - let tabs = windowState.get('tabs').splice(sourceFramePropsIndex, 1) if (newIndex > sourceFramePropsIndex) { newIndex-- } frames = frames.splice(newIndex, 0, action.sourceFrameProps) - tabs = tabs.splice(newIndex, 0, tabFromFrame(action.sourceFrameProps)) windowState = windowState.set('frames', frames) - windowState = windowState.set('tabs', tabs) // Since the tab could have changed pages, update the tab page as well windowState = updateTabPageIndex(windowState, frameStateUtil.getActiveFrame(windowState)) break @@ -444,11 +423,9 @@ const doAction = (action) => { case windowConstants.WINDOW_SET_THEME_COLOR: if (action.themeColor !== undefined) { windowState = windowState.setIn(frameStatePathForFrame(windowState, action.frameProps).concat(['themeColor']), action.themeColor) - windowState = windowState.setIn(tabStatePathForFrame(windowState, action.frameProps).concat(['themeColor']), action.themeColor) } if (action.computedThemeColor !== undefined) { windowState = windowState.setIn(frameStatePathForFrame(windowState, action.frameProps).concat(['computedThemeColor']), action.computedThemeColor) - windowState = windowState.setIn(tabStatePathForFrame(windowState, action.frameProps).concat(['computedThemeColor']), action.computedThemeColor) } break case windowConstants.WINDOW_FRAME_SHORTCUT_CHANGED: @@ -510,23 +487,19 @@ const doAction = (action) => { { const index = frameStateUtil.getFrameIndex(windowState, action.frameKey) windowState = windowState.setIn(['frames', index, 'audioMuted'], action.muted) - windowState = windowState.setIn(['tabs', index, 'audioMuted'], action.muted) } break case windowConstants.WINDOW_SET_ALL_AUDIO_MUTED: action.frameList.forEach((frameProp) => { let index = frameStateUtil.getFrameIndex(windowState, frameProp.frameKey) windowState = windowState.setIn(['frames', index, 'audioMuted'], frameProp.muted) - windowState = windowState.setIn(['tabs', index, 'audioMuted'], frameProp.muted) }) break case windowConstants.WINDOW_SET_AUDIO_PLAYBACK_ACTIVE: windowState = windowState.setIn(['frames', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'audioPlaybackActive'], action.audioPlaybackActive) - windowState = windowState.setIn(['tabs', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'audioPlaybackActive'], action.audioPlaybackActive) break case windowConstants.WINDOW_SET_FAVICON: windowState = windowState.setIn(['frames', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'icon'], action.favicon) - windowState = windowState.setIn(['tabs', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'icon'], action.favicon) break case windowConstants.WINDOW_SET_LAST_ZOOM_PERCENTAGE: windowState = windowState.setIn(['frames', frameStateUtil.getFramePropsIndex(frameStateUtil.getFrames(windowState), action.frameProps), 'lastZoomPercentage'], action.percentage) diff --git a/test/about/preferencesTest.js b/test/about/preferencesTest.js index 9196735a4be..736629742f0 100644 --- a/test/about/preferencesTest.js +++ b/test/about/preferencesTest.js @@ -82,9 +82,9 @@ describe('General Panel', function () { .waitForBrowserWindow() .waitUntil(function () { return this.getWindowState().then((val) => { - return (val.value.tabs.length === 2 && - val.value.tabs[0].location === page1 && - val.value.tabs[1].location === page2 + return (val.value.frames.length === 2 && + val.value.frames[0].location === page1 && + val.value.frames[1].location === page2 ) }) }) diff --git a/test/unit/app/renderer/components/tabs/content/audioTabIconTest.js b/test/unit/app/renderer/components/tabs/content/audioTabIconTest.js index c1ac8188a2e..7c70a607d8c 100644 --- a/test/unit/app/renderer/components/tabs/content/audioTabIconTest.js +++ b/test/unit/app/renderer/components/tabs/content/audioTabIconTest.js @@ -18,7 +18,7 @@ describe('Tabs content - AudioTabIcon', function () { it('should not show any audio icon if page has audio disabled', function () { const wrapper = shallow( frame.get('key') === frameProps.get('key')) assert.equal(false, inClosedFrames === undefined) }) it('sets isFullScreen=false for the removed frame', function () { frameProps = Immutable.fromJS({ key: 2, isFullScreen: true }) - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) const inClosedFrames = result.closedFrames.find((frame) => frame.get('key') === frameProps.get('key')) assert.equal(false, inClosedFrames.get('isFullScreen')) }) @@ -134,13 +126,13 @@ describe('frameStateUtil', function () { it('removed frame is NOT added to `closedFrames` if private', function () { frames = Immutable.fromJS([{ key: 2 }]) frameProps = Immutable.fromJS({ isPrivate: true, key: 2 }) - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) const inClosedFrames = result.closedFrames.find((frame) => frame.get('key') === frameProps.get('key')) assert.equal(true, inClosedFrames === undefined) }) it('removes the frame from `frames`', function () { - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) const inFrames = result.frames.find((frame) => frame.get('key') === frameProps.get('key')) assert.equal(true, inFrames === undefined) }) @@ -148,20 +140,19 @@ describe('frameStateUtil', function () { describe('does not change `activeFrameKey`', function () { it('if frame removed is not active and has parentFrameKey set', function () { frameProps = Immutable.fromJS({ key: 3 }) - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) assert.equal(activeFrameKey, result.activeFrameKey) }) it('if frame removed is not active and does NOT have parentFrameKey set', function () { frameProps = Immutable.fromJS({ key: 4 }) - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) assert.equal(activeFrameKey, result.activeFrameKey) }) it('if there are no frames left', function () { frames = Immutable.fromJS([{ key: 2 }]) - tabs = Immutable.fromJS([{ key: 2 }]) - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) assert.equal(activeFrameKey, result.activeFrameKey) }) }) @@ -176,13 +167,6 @@ describe('frameStateUtil', function () { {key: 5, lastAccessedTime: 1484075960}, {key: 6, lastAccessedTime: 1484075950} ]) - tabs = Immutable.fromJS([ - { key: 2 }, - { key: 3, pinnedLocation: 'https://www.facebook.com/' }, - { key: 4 }, - { key: 5 }, - { key: 6 } - ]) activeFrameKey = 4 frameProps = Immutable.fromJS({key: 4}) framePropsIndex = 2 @@ -191,7 +175,6 @@ describe('frameStateUtil', function () { it('parent tab action', function () { const result = frameStateUtil.removeFrame( frames, - tabs, closedFrames, frameProps, activeFrameKey, @@ -203,7 +186,6 @@ describe('frameStateUtil', function () { it('next tab action', function () { const result = frameStateUtil.removeFrame( frames, - tabs, closedFrames, frameProps, activeFrameKey, @@ -216,7 +198,6 @@ describe('frameStateUtil', function () { it('last active tab action', function () { const result = frameStateUtil.removeFrame( frames, - tabs, closedFrames, frameProps, activeFrameKey, @@ -241,7 +222,7 @@ describe('frameStateUtil', function () { }) activeFrameKey = 3 framePropsIndex = 1 - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) assert.equal(2, result.activeFrameKey) }) @@ -252,7 +233,7 @@ describe('frameStateUtil', function () { { pinnedLocation: 'https://www.facebook.com/', key: 4 } ]) framePropsIndex = 0 - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) assert.equal(4, result.activeFrameKey) }) @@ -262,7 +243,7 @@ describe('frameStateUtil', function () { { key: 2 } ]) framePropsIndex = 1 - const result = frameStateUtil.removeFrame(frames, tabs, closedFrames, frameProps, activeFrameKey, framePropsIndex) + const result = frameStateUtil.removeFrame(frames, closedFrames, frameProps, activeFrameKey, framePropsIndex) assert.equal(6, result.activeFrameKey) }) })