Keep per origin zoom settings

Fix #368

Auditors: @diracdeltas

docs/appActions.md

@@ -217,6 +217,20 @@ Changes an application level setting
 
 
 
+### changeSiteSetting(hostPattern, key, value) 
+
+Change a hostPattern's config
+
+**Parameters**
+
+**hostPattern**: `string`, The host pattern to update the config for
+
+**key**: `string`, The config key to update
+
+**value**: `string | number`, The value to update to
+
+
+
 
 * * *
 

docs/state.md

@@ -30,7 +30,9 @@ AppStore
     }
   }],
   siteSettings: {
-    [origin]: Object // settings object
+    [hostPattern]: {
+      zoomLevel: number
+    }
   },
   visits: [{
     location: string,
@@ -113,7 +115,6 @@ WindowStore
   activeFrameKey: number,
   previewFrameKey: number,
   frames: [{
-    zoomLevel: number, // current frame zoom level
     audioMuted: boolean, // frame is muted
     audioPlaybackActive: boolean, // frame is playing audio
     canGoBack: boolean,

docs/windowActions.md

@@ -561,6 +561,18 @@ Similar to setBlockedBy but for httpse redirects
 
 
 
+### inspectElement(x, y) 
+
+Inspect the element for the active webview at the x, y content position
+
+**Parameters**
+
+**x**: `number`, horizontal position of the element to inspect
+
+**y**: `number`, vertical position of the element to inspect
+
+
+
 
 * * *
 

js/actions/appActions.js

@@ -254,6 +254,21 @@ const appActions = {
       key,
       value
     })
+  },
+
+  /**
+   * Change a hostPattern's config
+   * @param {string} hostPattern - The host pattern to update the config for
+   * @param {string} key - The config key to update
+   * @param {string|number} value - The value to update to
+   */
+  changeSiteSetting: function (hostPattern, key, value) {
+    AppDispatcher.dispatch({
+      actionType: AppConstants.APP_CHANGE_SITE_SETTING,
+      hostPattern,
+      key,
+      value
+    })
   }
 }
 

js/actions/windowActions.js

@@ -786,29 +786,11 @@ const windowActions = {
     })
   },
 
-  zoomIn: function (frameProps) {
-    windowActions.zoom(frameProps, 0.5)
-  },
-
-  zoomOut: function (frameProps) {
-    windowActions.zoom(frameProps, -0.5)
-  },
-
-  zoom: function (frameProps, stepSize) {
-    dispatch({
-      frameProps,
-      stepSize,
-      actionType: WindowConstants.WINDOW_ZOOM
-    })
-  },
-
-  zoomReset: function (frameProps) {
-    dispatch({
-      frameProps,
-      actionType: WindowConstants.WINDOW_ZOOM_RESET
-    })
-  },
-
+  /**
+   * Inspect the element for the active webview at the x, y content position
+   * @param {number} x - horizontal position of the element to inspect
+   * @param {number} y - vertical position of the element to inspect
+   */
   inspectElement: function (x, y) {
     const webview = document.querySelector('.frameWrapper.isActive webview')
     if (webview) {

js/components/frame.js

@@ -10,6 +10,7 @@ const ImmutableComponent = require('./immutableComponent')
 const Immutable = require('immutable')
 const cx = require('../lib/classSet.js')
 const siteUtil = require('../state/siteUtil')
+const siteSettings = require('../state/siteSettings')
 const UrlUtil = require('../lib/urlutil')
 const messages = require('../constants/messages.js')
 const remote = global.require('electron').remote
@@ -85,6 +86,36 @@ class Frame extends ImmutableComponent {
 
   componentDidMount () {
     this.updateWebview()
+    if (this.zoomLevel !== config.zoom.defaultValue) {
+      // Timeout to work around setting zoom too early not working in Electron
+      setTimeout(() => {
+        this.webview.setZoomLevel(this.zoomLevel)
+      }, 1000)
+    }
+  }
+
+  zoom (stepSize) {
+    let newZoomLevel = this.zoomLevel
+    if (stepSize !== undefined &&
+        config.zoom.max >= this.zoomLevel + stepSize &&
+      config.zoom.min <= this.zoomLevel + stepSize) {
+      newZoomLevel += stepSize
+    } else {
+      newZoomLevel = config.zoom.defaultValue
+    }
+    appActions.changeSiteSetting(this.origin, 'zoomLevel', newZoomLevel)
+  }
+
+  zoomIn () {
+    this.zoom(config.zoom.step)
+  }
+
+  zoomOut () {
+    this.zoom(config.zoom.step * -1)
+  }
+
+  zoomReset () {
+    this.zoom()
   }
 
   componentDidUpdate (prevProps, prevState) {
@@ -140,13 +171,13 @@ class Frame extends ImmutableComponent {
         this.webview.loadURL(this.props.frame.get('location'))
         break
       case 'zoom-in':
-        windowActions.zoomIn(this.props.frame)
+        this.zoomIn()
         break
       case 'zoom-out':
-        windowActions.zoomOut(this.props.frame)
+        this.zoomOut()
         break
       case 'zoom-reset':
-        windowActions.zoomReset(this.props.frame)
+        this.zoomReset()
         break
       case 'toggle-dev-tools':
         if (this.webview.isDevToolsOpened()) {
@@ -435,6 +466,11 @@ class Frame extends ImmutableComponent {
     this.webview.goForward()
   }
 
+  get origin () {
+    const parsedUrl = urlParse(this.props.frame.get('location'))
+    return `${parsedUrl.protocol}//${parsedUrl.host}`
+  }
+
   onFocus () {
     windowActions.setTabPageIndexByFrame(this.props.frame)
     windowActions.setUrlBarActive(false)
@@ -449,9 +485,9 @@ class Frame extends ImmutableComponent {
 
   onUpdateWheelZoom () {
     if (this.wheelDeltaY > 0) {
-      windowActions.zoomIn(this.props.frame)
+      this.zoomIn()
     } else if (this.wheelDeltaY < 0) {
-      windowActions.zoomOut(this.props.frame)
+      this.zoomOut()
     }
     this.wheelDeltaY = 0
   }
@@ -491,10 +527,23 @@ class Frame extends ImmutableComponent {
       this.webview.setAudioMuted(false)
     }
 
-    let zoomLevel = nextProps.frame.get('zoomLevel')
-    if (zoomLevel !== this.props.frame.get('zoomLevel')) {
-      this.webview.setZoomLevel(zoomLevel)
+    const nextLocation = nextProps.frame.get('location')
+    const nextSiteSettings = siteSettings.getSiteSettingsForURL(nextProps.siteSettings, nextLocation)
+    if (nextSiteSettings) {
+      const nextZoom = nextSiteSettings.get('zoomLevel')
+      if (this.zoomLevel !== nextZoom) {
+        this.webview.setZoomLevel(nextZoom)
+      }
+    }
+  }
+
+  get zoomLevel () {
+    const location = this.props.frame.get('location')
+    const settings = siteSettings.getSiteSettingsForURL(this.props.siteSettings, location)
+    if (!settings) {
+      return config.zoom.defaultValue
     }
+    return settings.get('zoomLevel')
   }
 
   render () {

js/components/main.js

@@ -533,6 +533,7 @@ class Main extends ImmutableComponent {
                       .includes(siteTags.BOOKMARK_FOLDER)) || new Immutable.Map()
                 : null}
               passwords={this.props.appState.get('passwords')}
+              siteSettings={this.props.appState.get('siteSettings')}
               enableAds={this.enableAds}
               isPreview={frame.get('key') === this.props.windowState.get('previewFrameKey')}
               isActive={FrameStateUtil.isFrameKeyActive(this.props.windowState, frame.get('key'))}

js/constants/appConstants.js

@@ -24,7 +24,8 @@ const AppConstants = {
   APP_SET_RESOURCE_ENABLED: _,
   APP_UPDATE_LAST_CHECK: _,
   APP_SET_UPDATE_STATUS: _,
-  APP_CHANGE_SETTING: _
+  APP_CHANGE_SETTING: _,
+  APP_CHANGE_SITE_SETTING: _
 }
 
 module.exports = mapValuesByKeys(AppConstants)

js/constants/config.js

@@ -14,7 +14,7 @@ module.exports = {
     defaultValue: 0,
     min: -8,
     max: 9,
-    step: 1
+    step: 0.5
   },
   maxClosedFrames: 100,
   thumbnail: {

js/state/siteSettings.js

@@ -3,6 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const urlParse = require('url').parse
+const Immutable = require('immutable')
 
 /**
   * Obtains the site settings stored for a specific pattern.
@@ -15,13 +16,16 @@ module.exports.getSiteSettingsForHostPattern = (siteSettings, hostPattern) =>
   siteSettings.get(hostPattern)
 
 /**
-  * Set the settings object for the specified host pattern.
+  * Merges the settings for the specified host pattern.
   * @param {Object} siteSettings - The top level app state site settings indexed by hostPattern.
   * @param {string} hostPattern - The host pattern to merge into
-  * @param {Object} settingObj - An object of settings for the site
+  * @param {string} key - A setting key
+  * @param {string|number} value - A setting value
   */
-module.exports.setSiteSettings = (siteSettings, hostPattern, settingObj) =>
-  siteSettings.set(hostPattern, settingObj)
+module.exports.mergeSiteSetting = (siteSettings, hostPattern, key, value) =>
+  (siteSettings || Immutable.Map()).mergeIn([hostPattern], {
+    [key]: value
+  })
 
 /**
   * Remove all site settings for the specified hostPattern.
@@ -38,6 +42,9 @@ module.exports.removeSiteSettings = (siteSettings, hostPattern) =>
   * @return {Object} A merged settings object for the specified site setting or undefined
   */
 module.exports.getSiteSettingsForURL = (siteSettings, location) => {
+  if (!location || !siteSettings) {
+    return undefined
+  }
   // Example: https://www.brianbondy.com:8080/projects
   //   parsedUrl.host: www.brianbondy.com:8080
   //   parsedUrl.hostname: www.brianbondy.com
@@ -77,10 +84,10 @@ module.exports.getSiteSettingsForURL = (siteSettings, location) => {
 
   // Merge all the settingObj with the more specific first rules taking precedence
   const settingObj = settingObjs.reduce((mergedSettingObj, settingObj) =>
-    Object.assign(settingObj || {}, mergedSettingObj), {})
-  if (Object.keys(settingObj).length === 0) {
+    (settingObj || Immutable.Map()).merge(mergedSettingObj), Immutable.Map())
+  if (settingObj.size === 0) {
     return undefined
   }
-  return settingObj
+  return Immutable.fromJS(settingObj)
 }
 

js/stores/appStore.js

@@ -7,6 +7,7 @@ const AppConstants = require('../constants/appConstants')
 const appConfig = require('../constants/appConfig')
 const settings = require('../constants/settings')
 const siteUtil = require('../state/siteUtil')
+const siteSettings = require('../state/siteSettings')
 const electron = require('electron')
 const app = electron.app
 const ipcMain = electron.ipcMain
@@ -393,6 +394,10 @@ const handleAppAction = (action) => {
     case AppConstants.APP_CHANGE_SETTING:
       appState = appState.setIn(['settings', action.key], action.value)
       break
+    case AppConstants.APP_CHANGE_SITE_SETTING:
+      appState = appState.set('siteSettings',
+        siteSettings.mergeSiteSetting(appState.get('siteSettings'), action.hostPattern, action.key, action.value))
+      break
     default:
   }
 

js/stores/windowStore.js

@@ -2,7 +2,6 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-const config = require('../constants/config')
 const WindowDispatcher = require('../dispatcher/windowDispatcher')
 const EventEmitter = require('events').EventEmitter
 const WindowConstants = require('../constants/windowConstants')
@@ -472,22 +471,6 @@ const doAction = (action) => {
       let redirectedBy = windowState.getIn(redirectedByPath) || new Immutable.List()
       windowState = windowState.setIn(redirectedByPath, redirectedBy.push(action.location))
       break
-    // Zoom state
-    case WindowConstants.WINDOW_ZOOM:
-      let zoomLevel = FrameStateUtil.getFramePropValue(windowState, action.frameProps, 'zoomLevel')
-      // for backwards compatibility with previous stored window state
-      if (zoomLevel === undefined) {
-        zoomLevel = 1
-      }
-      if (config.zoom.max >= zoomLevel + action.stepSize &&
-          config.zoom.min <= zoomLevel + action.stepSize) {
-        zoomLevel += action.stepSize
-      }
-      windowState = windowState.setIn(FrameStateUtil.getFramePropPath(windowState, action.frameProps, 'zoomLevel'), zoomLevel)
-      break
-    case WindowConstants.WINDOW_ZOOM_RESET:
-      windowState = windowState.setIn(FrameStateUtil.getFramePropPath(windowState, action.frameProps, 'zoomLevel'), config.zoom.defaultValue)
-      break
     default:
   }