Making the Clear Browsing Data panel stateful

Majorly, the change affects 2 places in the UI. The Clear Browsing
Data… option in the History menu and the Clear Browsing Data Now…
button in the Security tab in Preferences. We store the state of the
toggled options when the user clears anytime, so that the next time
he/she opens this dialog, the options are pre-populated as per the last
selected options

From a technical perspective, we have moved away from saving the
current state of the SwitchControls in the windowState. We only save
the visibility on the windowState. The whole state is managed by the
panel itself and just updates the appState on clicking clear.

Fixes #4082

app/browser/menu.js

@@ -316,7 +316,7 @@ const createHistorySubmenu = () => {
       label: locale.translation('clearBrowsingData'),
       accelerator: 'Shift+CmdOrCtrl+Delete',
       click: function (item, focusedWindow) {
-        CommonMenu.sendToFocusedWindow(focusedWindow, [messages.SHORTCUT_OPEN_CLEAR_BROWSING_DATA_PANEL, {browserHistory: true}])
+        CommonMenu.sendToFocusedWindow(focusedWindow, [messages.SHORTCUT_OPEN_CLEAR_BROWSING_DATA_PANEL])
       }
     }
   ]

app/sessionStore.js

@@ -114,11 +114,10 @@ module.exports.cleanPerWindowData = (perWindowData, isShutdown) => {
   delete perWindowData.bookmarkDetail
   // Don't restore bravery panel
   delete perWindowData.braveryPanelDetail
-  // Don't restore cache clearing popup
-  delete perWindowData.clearBrowsingDataDetail
-  // Don't restore drag data
+  // Don't restore drag data and clearBrowsingDataPanel's visibility
   if (perWindowData.ui) {
     delete perWindowData.ui.dragging
+    delete perWindowData.ui.isClearBrowsingDataPanelVisible
   }
   perWindowData.frames = perWindowData.frames || []
   let newKey = 0

docs/appActions.md

@@ -92,7 +92,7 @@ Adds a site to the site list
 
 ### clearHistory() 
 
-Clears history (all sites without tags). Indirectly called by appActions.clearAppData().
+Clears history (all sites without tags). Indirectly called by appActions.onClearBrowsingData().
 
 
 
@@ -403,13 +403,13 @@ Adds information about pending basic auth login requests
 
 
 
-### clearAppData(clearDataDetail) 
+### onClearBrowsingData(clearDataDetail) 
 
-Clears the data specified in dataDetail
+Clears the data specified in clearDataDetail
 
 **Parameters**
 
-**clearDataDetail**: `object`, the app data to clear as per doc/state.md's clearBrowsingDataDetail
+**clearDataDetail**: `object`, the app data to clear as per doc/state.md's clearBrowsingDataDefaults
 
 
 

docs/state.md

@@ -266,7 +266,17 @@ AppStore
   menu: {
     template: object // used on Windows and by our tests: template object with Menubar control
   },
-  defaultBrowserCheckComplete: boolean // true to indicate default browser check is complete
+  defaultBrowserCheckComplete: boolean, // true to indicate default browser check is complete
+  clearBrowsingDataDefaults: {
+    browserHistory: boolean,
+    downloadHistory: boolean,
+    cachedImagesAndFiles: boolean,
+    savedPasswords: boolean,
+    allSiteCookies: boolean,
+    autocompleteData: boolean,
+    autofillData: boolean,
+    savedSiteSettings: booleanß
+  }
 }
 ```
 
@@ -392,6 +402,7 @@ WindowStore
     position: array, // last known window position [x, y]
     size: array, // last known window size [x, y]
     isFullScreen: boolean, // true if window is fullscreen
+    isClearBrowsingDataPanelVisible: boolean, // true if the Clear Browsing Data panel is visible
     mouseInTitlebar: boolean, //Whether or not the mouse is in the titlebar
     dragging: {
       dragType: string, // tab, bookmark
@@ -444,13 +455,6 @@ WindowStore
     expandNoScript: boolean, // Whether noscript section should be expanded
     expandFp: boolean // Whether fingerprinting protection should be expanded
   },
-  clearBrowsingDataDetail: {
-    browserHistory: boolean,
-    downloadHistory: boolean,
-    cachedImagesAndFiles: boolean,
-    savedPasswords: boolean,
-    allSiteCookies: boolean
-  },
   contextMenuDetail: {
     left: number, // the left position of the context menu
     right: number, // the right position of the context menu

docs/windowActions.md

@@ -765,9 +765,13 @@ Adds a history entry
 
 
 
-### setClearBrowsingDataDetail() 
+### setClearBrowsingDataPanelVisible(isVisible) 
 
-Sets the clear browsing data popup detail
+Sets whether the clear browsing data popup is visible
+
+**Parameters**
+
+**isVisible**: `boolean`, Sets whether the clear browsing data popup is visible
 
 
 

js/about/aboutActions.js

@@ -206,8 +206,8 @@ const aboutActions = {
     })
   },
 
-  clearBrowsingDataNow: function (clearBrowsingDataDetail) {
-    ipc.sendToHost(messages.CLEAR_BROWSING_DATA_NOW, clearBrowsingDataDetail)
+  clearBrowsingDataNow: function () {
+    ipc.sendToHost(messages.CLEAR_BROWSING_DATA_NOW)
   },
 
   importBrowserDataNow: function () {

js/about/preferences.js

@@ -1548,7 +1548,7 @@ class SecurityTab extends ImmutableComponent {
     this.clearBrowsingDataNow = this.clearBrowsingDataNow.bind(this)
   }
   clearBrowsingDataNow () {
-    aboutActions.clearBrowsingDataNow({browserHistory: true})
+    aboutActions.clearBrowsingDataNow()
   }
   onToggleFlash (e) {
     aboutActions.setResourceEnabled(flash, e.target.value)

js/actions/appActions.js

@@ -128,7 +128,7 @@ const appActions = {
   },
 
   /**
-   * Clears history (all sites without tags). Indirectly called by appActions.clearAppData().
+   * Clears history (all sites without tags). Indirectly called by appActions.onClearBrowsingData().
    */
   clearHistory: function () {
     AppDispatcher.dispatch({
@@ -492,12 +492,12 @@ const appActions = {
   },
 
   /**
-   * Clears the data specified in dataDetail
-   * @param {object} clearDataDetail - the app data to clear as per doc/state.md's clearBrowsingDataDetail
+   * Clears the data specified in clearDataDetail
+   * @param {object} clearDataDetail - the app data to clear as per doc/state.md's clearBrowsingDataDefaults
    */
-  clearAppData: function (clearDataDetail) {
+  onClearBrowsingData: function (clearDataDetail) {
     AppDispatcher.dispatch({
-      actionType: appConstants.APP_CLEAR_DATA,
+      actionType: appConstants.APP_ON_CLEAR_BROWSING_DATA,
       clearDataDetail
     })
   },

js/actions/windowActions.js

@@ -980,12 +980,13 @@ const windowActions = {
   },
 
   /**
-   * Sets the clear browsing data popup detail
+   * Sets whether the clear browsing data popup is visible
+   * @param {boolean} isVisible
    */
-  setClearBrowsingDataDetail: function (clearBrowsingDataDetail) {
+  setClearBrowsingDataPanelVisible: function (isVisible) {
     dispatch({
-      actionType: windowConstants.WINDOW_SET_CLEAR_BROWSING_DATA_DETAIL,
-      clearBrowsingDataDetail
+      actionType: windowConstants.WINDOW_SET_CLEAR_BROWSING_DATA_VISIBLE,
+      isVisible
     })
   },
 

js/components/clearBrowsingDataPanel.js

@@ -3,17 +3,16 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const React = require('react')
-const ImmutableComponent = require('./immutableComponent')
+const Immutable = require('immutable')
 const Dialog = require('./dialog')
 const Button = require('./button')
 const SwitchControl = require('./switchControl')
-const windowActions = require('../actions/windowActions')
 const appActions = require('../actions/appActions')
 const ipc = require('electron').ipcRenderer
 const messages = require('../constants/messages')
 
-class ClearBrowsingDataPanel extends ImmutableComponent {
-  constructor () {
+class ClearBrowsingDataPanel extends React.Component {
+  constructor (props) {
     super()
     this.onToggleBrowserHistory = this.onToggleSetting.bind(this, 'browserHistory')
     this.onToggleDownloadHistory = this.onToggleSetting.bind(this, 'downloadHistory')
@@ -24,14 +23,19 @@ class ClearBrowsingDataPanel extends ImmutableComponent {
     this.onToggleAutofillData = this.onToggleSetting.bind(this, 'autofillData')
     this.onToggleSavedSiteSettings = this.onToggleSetting.bind(this, 'savedSiteSettings')
     this.onClear = this.onClear.bind(this)
+    this.state = {
+      clearBrowsingDataDetail: props.clearBrowsingDataDefaults ? props.clearBrowsingDataDefaults : Immutable.Map()
+    }
   }
-  onToggleSetting (setting, e) {
-    windowActions.setClearBrowsingDataDetail(this.props.clearBrowsingDataDetail.set(setting, e.target.value))
+  onToggleSetting (setting) {
+    this.setState(({clearBrowsingDataDetail}) => ({
+      clearBrowsingDataDetail: clearBrowsingDataDetail.update(setting, isChecked => !isChecked)
+    }))
   }
   onClear () {
-    appActions.clearAppData(this.props.clearBrowsingDataDetail)
+    appActions.onClearBrowsingData(this.state.clearBrowsingDataDetail)
     this.props.onHide()
-    let detail = this.props.clearBrowsingDataDetail
+    let detail = this.state.clearBrowsingDataDetail
     if (detail.get('allSiteCookies') && detail.get('browserHistory') &&
         detail.get('cachedImagesAndFiles')) {
       ipc.send(messages.PREFS_RESTART)
@@ -42,14 +46,14 @@ class ClearBrowsingDataPanel extends ImmutableComponent {
       <div className='clearBrowsingData' onClick={(e) => e.stopPropagation()}>
         <div className='formSection clearBrowsingDataTitle' data-l10n-id='clearBrowsingData' />
         <div className='formSection clearBrowsingDataOptions'>
-          <SwitchControl className='browserHistorySwitch' rightl10nId='browserHistory' checkedOn={this.props.clearBrowsingDataDetail.get('browserHistory')} onClick={this.onToggleBrowserHistory} />
-          <SwitchControl rightl10nId='downloadHistory' checkedOn={this.props.clearBrowsingDataDetail.get('downloadHistory')} onClick={this.onToggleDownloadHistory} />
-          <SwitchControl rightl10nId='cachedImagesAndFiles' checkedOn={this.props.clearBrowsingDataDetail.get('cachedImagesAndFiles')} onClick={this.onToggleCachedImagesAndFiles} />
-          <SwitchControl rightl10nId='savedPasswords' checkedOn={this.props.clearBrowsingDataDetail.get('savedPasswords')} onClick={this.onToggleSavedPasswords} />
-          <SwitchControl rightl10nId='allSiteCookies' checkedOn={this.props.clearBrowsingDataDetail.get('allSiteCookies')} onClick={this.onToggleAllSiteCookies} />
-          <SwitchControl className='autocompleteDataSwitch' rightl10nId='autocompleteData' checkedOn={this.props.clearBrowsingDataDetail.get('autocompleteData')} onClick={this.onToggleAutocompleteData} />
-          <SwitchControl className='autofillDataSwitch' rightl10nId='autofillData' checkedOn={this.props.clearBrowsingDataDetail.get('autofillData')} onClick={this.onToggleAutofillData} />
-          <SwitchControl className='siteSettingsSwitch' rightl10nId='savedSiteSettings' checkedOn={this.props.clearBrowsingDataDetail.get('savedSiteSettings')} onClick={this.onToggleSavedSiteSettings} />
+          <SwitchControl className='browserHistorySwitch' rightl10nId='browserHistory' checkedOn={this.state.clearBrowsingDataDetail.get('browserHistory')} onClick={this.onToggleBrowserHistory} />
+          <SwitchControl rightl10nId='downloadHistory' checkedOn={this.state.clearBrowsingDataDetail.get('downloadHistory')} onClick={this.onToggleDownloadHistory} />
+          <SwitchControl rightl10nId='cachedImagesAndFiles' checkedOn={this.state.clearBrowsingDataDetail.get('cachedImagesAndFiles')} onClick={this.onToggleCachedImagesAndFiles} />
+          <SwitchControl rightl10nId='savedPasswords' checkedOn={this.state.clearBrowsingDataDetail.get('savedPasswords')} onClick={this.onToggleSavedPasswords} />
+          <SwitchControl rightl10nId='allSiteCookies' checkedOn={this.state.clearBrowsingDataDetail.get('allSiteCookies')} onClick={this.onToggleAllSiteCookies} />
+          <SwitchControl className='autocompleteDataSwitch' rightl10nId='autocompleteData' checkedOn={this.state.clearBrowsingDataDetail.get('autocompleteData')} onClick={this.onToggleAutocompleteData} />
+          <SwitchControl className='autofillDataSwitch' rightl10nId='autofillData' checkedOn={this.state.clearBrowsingDataDetail.get('autofillData')} onClick={this.onToggleAutofillData} />
+          <SwitchControl className='siteSettingsSwitch' rightl10nId='savedSiteSettings' checkedOn={this.state.clearBrowsingDataDetail.get('savedSiteSettings')} onClick={this.onToggleSavedSiteSettings} />
         </div>
         <div className='formSection clearBrowsingDataButtons'>
           <Button l10nId='cancel' className='whiteButton' onClick={this.props.onHide} />

js/components/frame.js

@@ -780,8 +780,8 @@ class Frame extends ImmutableComponent {
           }
           break
         case messages.CLEAR_BROWSING_DATA_NOW:
-          method = (clearBrowsingDataDetail) =>
-            windowActions.setClearBrowsingDataDetail(clearBrowsingDataDetail)
+          method = () =>
+            windowActions.setClearBrowsingDataPanelVisible(true)
           break
         case messages.AUTOFILL_SET_ADDRESS:
           method = (currentDetail, originalDetail) =>

js/components/main.js

@@ -630,7 +630,7 @@ class Main extends ImmutableComponent {
   }
 
   onHideClearBrowsingDataPanel () {
-    windowActions.setClearBrowsingDataDetail()
+    windowActions.setClearBrowsingDataPanelVisible(false)
   }
 
   onHideImportBrowserDataPanel () {
@@ -872,7 +872,7 @@ class Main extends ImmutableComponent {
     const siteInfoIsVisible = this.props.windowState.getIn(['ui', 'siteInfo', 'isVisible'])
     const braveShieldsDisabled = this.braveShieldsDisabled
     const braveryPanelIsVisible = !braveShieldsDisabled && this.props.windowState.get('braveryPanelDetail')
-    const clearBrowsingDataPanelIsVisible = this.props.windowState.get('clearBrowsingDataDetail')
+    const clearBrowsingDataPanelIsVisible = this.props.windowState.getIn(['ui', 'isClearBrowsingDataPanelVisible'])
     const importBrowserDataPanelIsVisible = this.props.windowState.get('importBrowserDataDetail')
     const widevinePanelIsVisible = this.props.windowState.getIn(['widevinePanelDetail', 'shown'])
     const autofillAddressPanelIsVisible = this.props.windowState.get('autofillAddressDetail')
@@ -1054,7 +1054,7 @@ class Main extends ImmutableComponent {
         {
          clearBrowsingDataPanelIsVisible
           ? <ClearBrowsingDataPanel
-            clearBrowsingDataDetail={this.props.windowState.get('clearBrowsingDataDetail')}
+            clearBrowsingDataDefaults={this.props.appState.get('clearBrowsingDataDefaults')}
             onHide={this.onHideClearBrowsingDataPanel} />
           : null
         }

js/constants/appConstants.js

@@ -37,7 +37,7 @@ const appConstants = {
   APP_CHANGE_SITE_SETTING: _,
   APP_REMOVE_SITE_SETTING: _,
   APP_CLEAR_SITE_SETTINGS: _,
-  APP_CLEAR_DATA: _,
+  APP_ON_CLEAR_BROWSING_DATA: _,
   APP_IMPORT_BROWSER_DATA: _,
   APP_UPDATE_LEDGER_INFO: _,
   APP_LEDGER_RECOVERY_SUCCEEDED: _,

js/constants/windowConstants.js

@@ -71,7 +71,7 @@ const windowConstants = {
   WINDOW_SET_SECURITY_STATE: _,
   WINDOW_SET_STATE: _,
   WINDOW_SET_LAST_ZOOM_PERCENTAGE: _,
-  WINDOW_SET_CLEAR_BROWSING_DATA_DETAIL: _,
+  WINDOW_SET_CLEAR_BROWSING_DATA_VISIBLE: _,
   WINDOW_SET_IMPORT_BROWSER_DATA_DETAIL: _,
   WINDOW_SET_IMPORT_BROWSER_DATA_SELECTED: _,
   WINDOW_SET_AUTOFILL_ADDRESS_DETAIL: _,

js/stores/appStore.js

@@ -686,7 +686,9 @@ const handleAppAction = (action) => {
     case appConstants.APP_CLEAR_RECOVERY:
       appState = appState.setIn(['ui', 'about', 'preferences', 'recoverySucceeded'], undefined)
       break
-    case appConstants.APP_CLEAR_DATA:
+    case appConstants.APP_ON_CLEAR_BROWSING_DATA:
+      // TODO: Maybe make storing this state optional?
+      appState = appState.set('clearBrowsingDataDefaults', action.clearDataDetail)
       if (action.clearDataDetail.get('browserHistory')) {
         handleAppAction({actionType: appConstants.APP_CLEAR_HISTORY})
         BrowserWindow.getAllWindows().forEach((wnd) => wnd.webContents.send(messages.CLEAR_CLOSED_FRAMES))

js/stores/windowStore.js

@@ -581,12 +581,8 @@ const doAction = (action) => {
         })
       }
       break
-    case windowConstants.WINDOW_SET_CLEAR_BROWSING_DATA_DETAIL:
-      if (!action.clearBrowsingDataDetail) {
-        windowState = windowState.delete('clearBrowsingDataDetail')
-      } else {
-        windowState = windowState.set('clearBrowsingDataDetail', Immutable.fromJS(action.clearBrowsingDataDetail))
-      }
+    case windowConstants.WINDOW_SET_CLEAR_BROWSING_DATA_VISIBLE:
+      windowState = windowState.setIn(['ui', 'isClearBrowsingDataPanelVisible'], action.isVisible)
       break
     case windowConstants.WINDOW_SET_IMPORT_BROWSER_DATA_DETAIL:
       if (!action.importBrowserDataDetail) {
@@ -772,10 +768,10 @@ ipc.on(messages.SHORTCUT_PREV_TAB, () => {
   emitChanges()
 })
 
-ipc.on(messages.SHORTCUT_OPEN_CLEAR_BROWSING_DATA_PANEL, (e, clearBrowsingDataDetail) => {
+ipc.on(messages.SHORTCUT_OPEN_CLEAR_BROWSING_DATA_PANEL, (e) => {
   doAction({
-    actionType: windowConstants.WINDOW_SET_CLEAR_BROWSING_DATA_DETAIL,
-    clearBrowsingDataDetail
+    actionType: windowConstants.WINDOW_SET_CLEAR_BROWSING_DATA_VISIBLE,
+    isVisible: true
   })
 })
 

test/about/newTabTest.js

@@ -169,7 +169,7 @@ describe('about:newtab tests', function () {
       })
 
       it('shows sites that have been visited', function * () {
-        yield this.app.client.clearAppData({browserHistory: true})
+        yield this.app.client.onClearBrowsingData({browserHistory: true})
 
         yield loadPageWithTracker(this.app.client)
 
@@ -180,7 +180,7 @@ describe('about:newtab tests', function () {
       })
 
       it('lets you pin a tile (and shows the pinned icon afterwards)', function * () {
-        yield this.app.client.clearAppData({browserHistory: true})
+        yield this.app.client.onClearBrowsingData({browserHistory: true})
 
         yield loadPageWithTracker(this.app.client)
 
@@ -196,7 +196,7 @@ describe('about:newtab tests', function () {
       })
 
       it('doesn\'t show about pages on topSites grid', function * () {
-        yield this.app.client.clearAppData({browserHistory: true})
+        yield this.app.client.onClearBrowsingData({browserHistory: true})
 
         // Adding about pages shouldn't add them to topSites grid
         yield addDemoAboutPages(this.app.client)
@@ -223,7 +223,7 @@ describe('about:newtab tests', function () {
       it('shows favicon image for topSites', function * () {
         const pageWithFavicon = Brave.server.url('favicon.html')
         yield this.app.client
-          .clearAppData({browserHistory: true})
+          .onClearBrowsingData({browserHistory: true})
           .tabByUrl(Brave.newTabUrl)
           .url(pageWithFavicon)
           .waitForUrl(pageWithFavicon)
@@ -236,7 +236,7 @@ describe('about:newtab tests', function () {
       })
 
       it('replace topSites favicon images with a letter when no icon is found', function * () {
-        yield this.app.client.clearAppData({browserHistory: true})
+        yield this.app.client.onClearBrowsingData({browserHistory: true})
 
         const pageWithoutFavicon = Brave.server.url('page_favicon_not_found.html')
 

test/app/sessionStoreTest.js

@@ -16,7 +16,7 @@ describe('sessionStore', function () {
       yield Brave.startApp()
       yield setup(Brave.app.client)
       yield Brave.app.client
-        .clearAppData({browserHistory: true})
+        .onClearBrowsingData({browserHistory: true})
         .waitForUrl(Brave.newTabUrl)
         .loadUrl(page1Url)
         .windowParentByUrl(page1Url)