Merge pull request #6026 from maaz93/new-history-state

Making the Clear Browsing Data panel stateful

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ß
+  }
 }
 ```
 
@@ -391,6 +401,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
@@ -443,13 +454,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

@@ -739,9 +739,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

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

@@ -960,12 +960,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

@@ -745,8 +745,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

@@ -631,7 +631,7 @@ class Main extends ImmutableComponent {
   }
 
   onHideClearBrowsingDataPanel () {
-    windowActions.setClearBrowsingDataDetail()
+    windowActions.setClearBrowsingDataPanelVisible(false)
   }
 
   onHideImportBrowserDataPanel () {
@@ -873,7 +873,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')
@@ -1053,7 +1053,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

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

@@ -751,12 +751,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) {
@@ -938,10 +934,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)