Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Set zoomPercent on appState for each tab
Browse files Browse the repository at this point in the history
Fix #14045
Previously this was set on window state, but was not used, possibly because it was not reliable as it did not get the initial value which is set internally in muon based on origin. Instead, when this was working, it was read directly from the webview, which we try to avoid due to the webview's multi-purpose nature. For example, the webview may not be displaying the active tab - it may be displaying a preview, or waiting to display the active tab asynchronously.
  • Loading branch information
petemill committed May 8, 2018
1 parent 3c235d7 commit cd7f185
Show file tree
Hide file tree
Showing 11 changed files with 22 additions and 45 deletions.
12 changes: 11 additions & 1 deletion app/browser/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const getTabValue = function (tabId) {
tabValue = tabValue.set('partition', tab.session.partition)
tabValue = tabValue.set('partitionNumber', getPartitionNumber(tab.session.partition))
tabValue = tabValue.set('isPlaceholder', tab.isPlaceholder())
tabValue = tabValue.set('zoomPercent', tab.getZoomPercent())
return tabValue.set('tabId', tabId)
}
}
Expand Down Expand Up @@ -633,6 +634,15 @@ const api = {
appActions.tabMoved(tabId)
})

let lastZoomPercent = 100
tab.on('preferred-size-changed', debounce(() => {
const zoomPercent = tab.getZoomPercent()
if (zoomPercent !== lastZoomPercent) {
lastZoomPercent = zoomPercent
updateTab(tabId, { zoomPercent })
}
}, 100))

tab.on('will-attach', (e, windowWebContents) => {
if (shouldDebugTabEvents) {
console.log('will-attach', windowWebContents.id, tab.tabValue().windowId)
Expand Down Expand Up @@ -1421,7 +1431,7 @@ const api = {
// communicate those out here.
if (newTabValue && oldTabValue) {
const changeInfo = {}
const rendererAwareProps = ['index', 'pinned', 'url', 'active']
const rendererAwareProps = ['index', 'pinned', 'url', 'active', 'zoomPercent']
rendererAwareProps.forEach((prop) => {
const newPropVal = newTabValue.get(prop)
if (oldTabValue.get(prop) !== newPropVal) {
Expand Down
9 changes: 6 additions & 3 deletions app/renderer/components/common/contextMenu/contextMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@ const keyCodes = require('../../../../common/constants/keyCodes')

// State
const contextMenuState = require('../../../../common/state/contextMenuState')
const tabState = require('../../../../common/state/tabState')
const appStore = require('../../../../../js/stores/appStoreRenderer')
const { getCurrentWindowId } = require('../../../currentWindow')

// Utils
const frameStateUtil = require('../../../../../js/state/frameStateUtil')
const {separatorMenuItem} = require('../../../../common/commonMenu')
const {wrappingClamp} = require('../../../../common/lib/formatUtil')

Expand Down Expand Up @@ -219,12 +221,13 @@ class ContextMenu extends React.Component {

mergeProps (state, ownProps) {
const currentWindow = state.get('currentWindow')
const activeFrame = frameStateUtil.getActiveFrame(currentWindow) || Immutable.Map()

const selectedIndex = currentWindow.getIn(['ui', 'contextMenu', 'selectedIndex'], null)
const contextMenuDetail = contextMenuState.getContextMenu(currentWindow)

const props = {}
props.lastZoomPercentage = activeFrame.get('lastZoomPercentage')
const activeTab = tabState.getActiveTab(appStore.state, getCurrentWindowId())
props.lastZoomPercentage = activeTab && activeTab.get('zoomPercent')
props.contextMenuDetail = contextMenuDetail // TODO (nejc) only primitives
props.selectedIndex = typeof selectedIndex === 'object' &&
Array.isArray(selectedIndex) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,7 @@ class ContextMenuItem extends ImmutableComponent {
return label
}
if (item.get('labelDataBind') === 'zoomLevel') {
const activeWebview = document.querySelector('.frameWrapper.isActive webview')
let percent = 100
if (activeWebview) {
percent = activeWebview.getZoomPercent()
}
const percent = this.props.lastZoomPercentage || 100
return `${percent}%`
}
return ''
Expand Down
7 changes: 1 addition & 6 deletions app/renderer/components/frame/guestInstanceRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ class GuestInstanceRenderer extends React.Component {
classNameWebview: css(styles.guestInstanceRenderer__webview),
classNameWebviewAttached: css(styles.guestInstanceRenderer__webview_attached),
classNameWebviewAttaching: css(styles.guestInstanceRenderer__webview_attaching),
onFocus: this.onFocus.bind(this),
onZoomChange: this.onUpdateZoom.bind(this)
onFocus: this.onFocus.bind(this)
})
webviewActions.init(this.webviewDisplay)
// treat the container as main frame position for mouse position
Expand All @@ -122,10 +121,6 @@ class GuestInstanceRenderer extends React.Component {
}
}

onUpdateZoom (zoomPercent) {
windowActions.setLastZoomPercentage(this.props.frameKey, zoomPercent)
}

render () {
const debugInfo = this.props.displayDebugInfo
? `WindowId: ${getCurrentWindowId()}, TabId: ${this.props.tabId}, GuestId: ${this.props.guestInstanceId}, FrameKey: ${this.props.frameKey}, guestIsReady: ${this.props.guestIsReady}, frameIsInWindow: ${this.props.frameIsInWindow}, activeFrameKey: ${this.props.activeFrameKey}, windowIsFocused: ${this.props.windowIsFocused}`
Expand Down
6 changes: 0 additions & 6 deletions app/renderer/rendererShortcutHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,7 @@ function handleShortcut (frameKey, shortcut, e, args) {
const tabId = frameStateUtil.getTabIdByFrameKey(windowStore.state, frameKey)
getWebContents(tabId, (webContents) => {
if (webContents) {
const frame = frameStateUtil.getFrameByKey(windowStore.state, frameKey)
webContents.zoomIn()
windowActions.setLastZoomPercentage(frame, webContents.getZoomPercent())
}
})
break
Expand All @@ -102,9 +100,7 @@ function handleShortcut (frameKey, shortcut, e, args) {
const tabId = frameStateUtil.getTabIdByFrameKey(windowStore.state, frameKey)
getWebContents(tabId, (webContents) => {
if (webContents) {
const frame = frameStateUtil.getFrameByKey(windowStore.state, frameKey)
webContents.zoomOut()
windowActions.setLastZoomPercentage(frame, webContents.getZoomPercent())
}
})
break
Expand All @@ -113,9 +109,7 @@ function handleShortcut (frameKey, shortcut, e, args) {
const tabId = frameStateUtil.getTabIdByFrameKey(windowStore.state, frameKey)
getWebContents(tabId, (webContents) => {
if (webContents) {
const frame = frameStateUtil.getFrameByKey(windowStore.state, frameKey)
webContents.zoomReset()
windowActions.setLastZoomPercentage(frame, webContents.getZoomPercent())
}
})
break
Expand Down
4 changes: 2 additions & 2 deletions docs/state.md
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,8 @@ AppStore
suppress: boolean, // if true, show a suppress checkbox (defaulted to not checked)
title: string, // title is the source; ex: "brave.com says:"
},
muted: boolean, // is the tab muted
muted: boolean, // is the tab muted,
zoomPercent: number, // current zoom levellast
windowId: number // the windowId that contains the tab
guestInstanceId: number,
tabId: number
Expand Down Expand Up @@ -739,7 +740,6 @@ WindowStore
isPrivate: boolean, // private browsing tab
key: number,
lastAccessedTime: datetime,
lastZoomPercentage: number, // last value that was used for zooming
loading: boolean,
location: string, // the currently navigated location
modalPromptDetail: object,
Expand Down
15 changes: 0 additions & 15 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -579,21 +579,6 @@ const windowActions = {
})
},

/**
* Dispatches a message to store the last zoom percentage.
* This is mainly just used to trigger updates throughout React.
*
* @param {object} frameKey - The frame to set blocked info on
* @param {number} percentage - The new zoom percentage
*/
setLastZoomPercentage: function (frameKey, percentage) {
dispatch({
actionType: windowConstants.WINDOW_SET_LAST_ZOOM_PERCENTAGE,
frameKey,
percentage
})
},

/**
* Saves the position of the window in the window state
* @param {Array} position - [x, y]
Expand Down
1 change: 0 additions & 1 deletion js/constants/windowConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ const windowConstants = {
WINDOW_SET_REDIRECTED_BY: _, // Whether or not to show site info like redirected resources
WINDOW_SET_SECURITY_STATE: _,
WINDOW_SET_STATE: _,
WINDOW_SET_LAST_ZOOM_PERCENTAGE: _,
WINDOW_SET_CLEAR_BROWSING_DATA_VISIBLE: _,
WINDOW_SET_IMPORT_BROWSER_DATA_DETAIL: _,
WINDOW_SET_IMPORT_BROWSER_DATA_SELECTED: _,
Expand Down
3 changes: 0 additions & 3 deletions js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -522,9 +522,6 @@ const doAction = (action) => {
case windowConstants.WINDOW_SET_FAVICON:
windowState = windowState.setIn(['frames', frameStateUtil.getFrameIndex(windowState, action.frameProps.get('key')), 'icon'], action.favicon)
break
case windowConstants.WINDOW_SET_LAST_ZOOM_PERCENTAGE:
windowState = windowState.setIn(['frames', frameStateUtil.getFrameIndex(windowState, action.frameKey), 'lastZoomPercentage'], action.percentage)
break
case windowConstants.WINDOW_SET_MOUSE_IN_TITLEBAR:
windowState = windowState.setIn(['ui', 'mouseInTitlebar'], action.mouseInTitlebar)
break
Expand Down
1 change: 0 additions & 1 deletion test/unit/app/common/lib/historyUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,6 @@ describe('historyUtil unit tests', function () {
isFullScreen: false,
isPrivate: false,
key: 1,
lastZoomPercentage: 1,
loading: true,
location: 'https://brave.com',
title: 'Brave'
Expand Down
3 changes: 1 addition & 2 deletions test/unit/app/renderer/components/frame/frameTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ describe.skip('Frame component unit tests', function () {
const fakeWindowActions = {
frameShortcutChanged: () => {},
setFindbarShown: () => {},
setActiveFrame: () => {},
setLastZoomPercentage: () => {}
setActiveFrame: () => {}
}

const domUtil = {
Expand Down

0 comments on commit cd7f185

Please sign in to comment.