Set zoomPercent on appState for each tab

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.
Since there is not yet a muon Tab event for 'zoom-changed', we must manually update the zoomPercent tabState property every time we knowingly change the zoom level.

app/browser/reducers/tabsReducer.js

@@ -143,6 +143,13 @@ const tabsReducer = (state, action, immutableAction) => {
         })
         break
       }
+    case tabActionConsts.ZOOM_CHANGED:
+      {
+        const tabId = tabState.resolveTabId(state, action.get('tabId'))
+        const zoomPercent = action.get('zoomPercent')
+        state = tabState.setZoomPercent(state, tabId, zoomPercent)
+        break
+      }
     case appConstants.APP_SET_STATE:
       state = tabs.init(state, action)
       break

app/browser/tabs.js

@@ -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)
   }
 }
@@ -1421,7 +1422,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) {

app/common/actions/tabActions.js

@@ -51,6 +51,13 @@ const tabActions = {
     dispatchAction(tabActionConstants.STOP_FIND_IN_PAGE_REQUEST, {
       tabId
     })
+  },
+
+  zoomChanged (tabId, zoomPercent) {
+    dispatchAction(tabActionConstants.ZOOM_CHANGED, {
+      tabId,
+      zoomPercent
+    })
   }
 }
 

app/common/constants/tabAction.js

@@ -7,5 +7,6 @@
    FINISH_NAVIGATION: 'tabActionsDidFinishNavigation',
    START_NAVIGATION: 'tabActionsDidStartNavigation',
    FIND_IN_PAGE_REQUEST: 'tabActionsFindInPageRequest',
-   STOP_FIND_IN_PAGE_REQUEST: 'tabActionsStopFindInPageRequest'
+   STOP_FIND_IN_PAGE_REQUEST: 'tabActionsStopFindInPageRequest',
+   ZOOM_CHANGED: 'tabActionsZoomChanged'
  }

app/common/state/tabState.js

@@ -680,6 +680,20 @@ const tabState = {
     }
     // handle set window
     return state.setIn(path, windowId)
+  },
+
+  setZoomPercent: (state, tabId, zoomPercent) => {
+    let path = tabState.getPathByTabId(state, tabId)
+    if (!path) {
+      console.error(`setZoomPercent: tab with ID ${tabId} not found!`)
+      return state
+    }
+    if (typeof zoomPercent !== 'number') {
+      console.error(`setZoomPercent: bad value for zoomPercent: ${zoomPercent}`)
+      return state
+    }
+    path = [...path, 'zoomPercent']
+    return state.setIn(path, zoomPercent)
   }
 }
 

app/renderer/components/common/contextMenu/contextMenu.js

@@ -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')
 
@@ -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) &&

app/renderer/components/common/contextMenu/contextMenuItem.js

@@ -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 ''

app/renderer/components/frame/guestInstanceRenderer.js

@@ -11,6 +11,7 @@ const WebviewDisplay = require('../../pooledWebviewDisplay')
 // Actions
 const windowActions = require('../../../../js/actions/windowActions')
 const webviewActions = require('../../../../js/actions/webviewActions')
+const tabActions = require('../../../common/actions/tabActions')
 
 // state
 const frameStateUtil = require('../../../../js/state/frameStateUtil')
@@ -101,8 +102,8 @@ 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)
+        onZoomChange: this.onUpdateZoom.bind(this),
+        onFocus: this.onFocus.bind(this)
       })
       webviewActions.init(this.webviewDisplay)
       // treat the container as main frame position for mouse position
@@ -123,7 +124,9 @@ class GuestInstanceRenderer extends React.Component {
   }
 
   onUpdateZoom (zoomPercent) {
-    windowActions.setLastZoomPercentage(this.props.frameKey, zoomPercent)
+    // TODO: better to respond to a muon Tab event `zoom-changed` via ZoomObserver
+    // if that is provided in the future
+    tabActions.zoomChanged(this.props.tabId, zoomPercent)
   }
 
   render () {

app/renderer/rendererShortcutHandler.js

@@ -91,9 +91,10 @@ 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())
+          // TODO: better to respond to a muon Tab event `zoom-changed` via ZoomObserver
+          // if that is provided in the future
+          tabActions.zoomChanged(tabId, webContents.getZoomPercent())
         }
       })
       break
@@ -102,9 +103,10 @@ 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())
+          // TODO: better to respond to a muon Tab event `zoom-changed` via ZoomObserver
+          // if that is provided in the future
+          tabActions.zoomChanged(tabId, webContents.getZoomPercent())
         }
       })
       break
@@ -113,9 +115,10 @@ 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())
+          // TODO: better to respond to a muon Tab event `zoom-changed` via ZoomObserver
+          // if that is provided in the future
+          tabActions.zoomChanged(tabId, webContents.getZoomPercent())
         }
       })
       break

docs/state.md

@@ -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
@@ -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,

js/actions/windowActions.js

@@ -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]

js/constants/windowConstants.js

@@ -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: _,

js/stores/windowStore.js

@@ -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

test/unit/app/browser/tabsTest.js

@@ -81,6 +81,7 @@ describe('tabs API unit tests', function () {
         once: (event, cb) => {
           setImmediate(cb)
         },
+        getZoomPercent: () => 100,
         isPlaceholder: () => false
       }
       if (tabId === 1) {

test/unit/app/common/lib/historyUtilTest.js

@@ -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'

test/unit/app/renderer/components/frame/frameTest.js

@@ -47,8 +47,7 @@ describe.skip('Frame component unit tests', function () {
   const fakeWindowActions = {
     frameShortcutChanged: () => {},
     setFindbarShown: () => {},
-    setActiveFrame: () => {},
-    setLastZoomPercentage: () => {}
+    setActiveFrame: () => {}
   }
 
   const domUtil = {

test/unit/lib/fakeTab.js

@@ -18,6 +18,7 @@ function FakeTab (id, windowId, guestInstanceId = nextGuestInstanceId++) {
   this._canGoBack = false
   this._canGoForward = false
   this._isPlaceholder = false
+  this._zoomPercent = 100
 }
 
 util.inherits(FakeTab, EventEmitter)
@@ -51,4 +52,8 @@ proto.isPlaceholder = function () {
   return this._isPlaceholder
 }
 
+proto.getZoomPercent = function () {
+  return this._zoomPercent
+}
+
 module.exports = FakeTab