Added default download path

Resolves #2110

Auditors: @bradleyrichter @bsclifton

Test Plan:
- go to preferences, general tab
- define your default download path
- toggle show dialog
- based on your settings, download dialog should be opened or not
- if you set don't show dialog, filse should be downloaded to your default set path

 # Added hover effect

app/browser/reducers/downloadsReducer.js

@@ -6,17 +6,20 @@
 
 const appConstants = require('../../../js/constants/appConstants')
 const downloadStates = require('../../../js/constants/downloadStates')
-const {clipboard, BrowserWindow, shell} = require('electron')
+const settings = require('../../../js/constants/settings')
+const {clipboard, BrowserWindow, shell, dialog, app} = require('electron')
 const fs = require('fs')
 const path = require('path')
 const {cancelDownload, pauseDownload, resumeDownload} = require('../electronDownloadItem')
 const {CANCEL, PAUSE, RESUME} = require('../../common/constants/electronDownloadItemActions')
+const appActions = require('../../../js/actions/appActions')
 
 const downloadsReducer = (state, action) => {
   const download = action.downloadId ? state.getIn(['downloads', action.downloadId]) : undefined
   if (!download &&
       ![appConstants.APP_MERGE_DOWNLOAD_DETAIL,
-        appConstants.APP_CLEAR_COMPLETED_DOWNLOADS].includes(action.actionType)) {
+        appConstants.APP_CLEAR_COMPLETED_DOWNLOADS,
+        appConstants.APP_DOWNLOAD_DEFAULT_PATH].includes(action.actionType)) {
     return state
   }
   switch (action.actionType) {
@@ -89,6 +92,18 @@ const downloadsReducer = (state, action) => {
         state = state.set('downloads', downloads)
       }
       break
+    case appConstants.APP_DOWNLOAD_DEFAULT_PATH:
+      const focusedWindow = BrowserWindow.getFocusedWindow()
+
+      dialog.showOpenDialog(focusedWindow, {
+        defaultPath: app.getPath('downloads'),
+        properties: ['openDirectory']
+      }, (folder) => {
+        if (Array.isArray(folder) && fs.lstatSync(folder[0]).isDirectory()) {
+          appActions.changeSetting(settings.DOWNLOAD_DEFAULT_PATH, folder[0])
+        }
+      })
+      break
   }
   return state
 }

app/extensions/brave/locales/en-US/preferences.properties

@@ -176,6 +176,8 @@ newTabDefaultSearchEngine=Default search engine
 newTabEmpty=Blank page
 myHomepage=My home page is
 multipleHomePages.title=Multiple home pages
+downloadDefaultPath=Save my downloads here:
+downloadAlwaysAsk=Always ask me where to save files
 aboutBlank=about:blank
 default=Default
 searchEngine=Search Engine

app/filtering.js

@@ -501,8 +501,8 @@ function registerForDownloadListener (session) {
       itemFilename = item.getFilename()
     }
 
-    const defaultPath = path.join(getSetting(settings.DEFAULT_DOWNLOAD_SAVE_PATH) || app.getPath('downloads'), itemFilename)
-    const savePath = (process.env.SPECTRON ? defaultPath : dialog.showSaveDialog(win, { defaultPath }))
+    const defaultPath = path.join(getSetting(settings.DOWNLOAD_DEFAULT_PATH) || getSetting(settings.DEFAULT_DOWNLOAD_SAVE_PATH) || app.getPath('downloads'), itemFilename)
+    const savePath = ((process.env.SPECTRON || !getSetting(settings.DOWNLOAD_ALWAYS_ASK)) ? defaultPath : dialog.showSaveDialog(win, { defaultPath }))
 
     // User cancelled out of save dialog prompt
     if (!savePath) {

app/renderer/components/settings.js

@@ -140,9 +140,71 @@ class SiteSettingCheckbox extends ImmutableComponent {
   }
 }
 
+class SettingItemIcon extends ImmutableComponent {
+  render () {
+    const bg = StyleSheet.create({
+      Icon: {
+        '-webkit-mask-image': `url(${this.props.icon})`
+      }
+    })
+
+    return <div className='settingItem'>
+      {
+        this.props.dataL10nId
+          ? <span data-l10n-id={this.props.dataL10nId} />
+          : null
+      }
+      <div>
+        {
+          this.props.position === 'left'
+            ? <span className={css(bg.Icon, styles.icon, styles.iconLeft)} data-icon-position='left' onClick={this.props.clickAction} />
+            : null
+        }
+        <div className={css(styles.child)}>
+          {this.props.children}
+        </div>
+        {
+          this.props.position === 'right'
+            ? <span className={css(bg.Icon, styles.icon, styles.iconRight)} data-icon-position='right' onClick={this.props.clickAction} />
+            : null
+        }
+      </div>
+    </div>
+  }
+}
+
+const styles = StyleSheet.create({
+  icon: {
+    backgroundColor: '#5a5a5a',
+    height: '16px',
+    width: '16px',
+    display: 'inline-block',
+    verticalAlign: 'top',
+    padding: '0px',
+    '-webkit-mask-repeat': 'no-repeat',
+
+    ':hover': {
+      backgroundColor: '#000'
+    }
+  },
+
+  iconLeft: {
+    margin: '8px 10px 0 0'
+  },
+
+  iconRight: {
+    margin: '8px 0 0 10px'
+  },
+
+  child: {
+    display: 'inline-block'
+  }
+})
+
 module.exports = {
   SettingsList,
   SettingItem,
   SettingCheckbox,
-  SiteSettingCheckbox
+  SiteSettingCheckbox,
+  SettingItemIcon
 }

docs/appActions.md

@@ -792,6 +792,12 @@ Action triggered by un-registering navigation handler
 
 
 
+### defaultDownloadPath() 
+
+Open dialog for default download path setting
+
+
+
 
 * * *
 

js/about/preferences.js

@@ -10,7 +10,7 @@ const UrlUtil = require('../lib/urlutil')
 
 // Components
 const PreferenceNavigation = require('../../app/renderer/components/preferences/preferenceNavigation')
-const {SettingsList, SettingItem, SettingCheckbox} = require('../../app/renderer/components/settings')
+const {SettingsList, SettingItem, SettingCheckbox, SettingItemIcon} = require('../../app/renderer/components/settings')
 const {SettingTextbox} = require('../../app/renderer/components/textbox')
 const {SettingDropdown} = require('../../app/renderer/components/dropdown')
 const Button = require('../components/button')
@@ -32,6 +32,7 @@ const {passwordManagers, extensionIds} = require('../constants/passwordManagers'
 const {startsWithOption, newTabMode, bookmarksToolbarMode, tabCloseAction, fullscreenOption} = require('../../app/common/constants/settingsEnums')
 
 const aboutActions = require('./aboutActions')
+const appActions = require('../actions/appActions')
 const getSetting = require('../settings').getSetting
 const SortableTable = require('../components/sortableTable')
 const searchProviders = require('../data/searchProviders')
@@ -115,6 +116,10 @@ class GeneralTab extends ImmutableComponent {
     return keyArray.every((key) => getSetting(key, this.props.settings) === true)
   }
 
+  openDownloadDialog () {
+    appActions.defaultDownloadPath()
+  }
+
   render () {
     const languageOptions = this.props.languageCodes.map(function (lc) {
       return (
@@ -184,6 +189,17 @@ class GeneralTab extends ImmutableComponent {
           isDarwin ? null : <SettingCheckbox dataL10nId='autoHideMenuBar' prefKey={settings.AUTO_HIDE_MENU} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
         }
         <SettingCheckbox dataL10nId='disableTitleMode' prefKey={settings.DISABLE_TITLE_MODE} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
+        <SettingItemIcon dataL10nId='downloadDefaultPath' position='right' icon={require('../../img/icon_pencil.svg')} clickAction={this.openDownloadDialog}>
+          <SettingTextbox
+            spellCheck='false'
+            readOnly='true'
+            data-l10n-id='downloadDefaultPathInput'
+            value={getSetting(settings.DOWNLOAD_DEFAULT_PATH, this.props.settings)}
+            onClick={this.openDownloadDialog} />
+        </SettingItemIcon>
+        <SettingCheckbox dataL10nId='downloadAlwaysAsk' prefKey={settings.DOWNLOAD_ALWAYS_ASK}
+          settings={this.props.settings}
+          onChangeSetting={this.props.onChangeSetting} />
         <SettingItem dataL10nId='bookmarkToolbarSettings'>
           <SettingDropdown id='bookmarksBarSelect' value={getSetting(settings.BOOKMARKS_TOOLBAR_MODE, this.props.settings)}
             onChange={changeSetting.bind(null, this.props.onChangeSetting, settings.BOOKMARKS_TOOLBAR_MODE)}>

js/actions/appActions.js

@@ -970,6 +970,15 @@ const appActions = {
       protocol,
       location
     })
+  },
+
+  /**
+   * Open dialog for default download path setting
+   */
+  defaultDownloadPath: function () {
+    AppDispatcher.dispatch({
+      actionType: appConstants.APP_DOWNLOAD_DEFAULT_PATH
+    })
   }
 }
 

js/constants/appConfig.js

@@ -127,6 +127,8 @@ module.exports = {
     'general.useragent.value': null, // Set at runtime
     'general.autohide-menu': true,
     'general.check-default-on-startup': true,
+    'general.download-default-path': '',
+    'general.download-always-ask': true,
     'search.default-search-engine': 'Google',
     'search.offer-search-suggestions': false, // false by default for privacy reasons
     'tabs.switch-to-new-tabs': false,

js/constants/appConstants.js

@@ -80,6 +80,7 @@ const appConstants = {
   APP_DOWNLOAD_DELETED: _,
   APP_DOWNLOAD_CLEARED: _,
   APP_DOWNLOAD_REDOWNLOADED: _,
+  APP_DOWNLOAD_DEFAULT_PATH: _,
   APP_SHOW_DOWNLOAD_DELETE_CONFIRMATION: _,
   APP_HIDE_DOWNLOAD_DELETE_CONFIRMATION: _,
   APP_ALLOW_FLASH_ONCE: _,

js/constants/settings.js

@@ -16,6 +16,8 @@ const settings = {
   LANGUAGE: 'general.language',
   CHECK_DEFAULT_ON_STARTUP: 'general.check-default-on-startup',
   IS_DEFAULT_BROWSER: 'general.is-default-browser',
+  DOWNLOAD_DEFAULT_PATH: 'general.download-default-path',
+  DOWNLOAD_ALWAYS_ASK: 'general.download-always-ask',
   // Search tab
   DEFAULT_SEARCH_ENGINE: 'search.default-search-engine',
   OFFER_SEARCH_SUGGESTIONS: 'search.offer-search-suggestions',

test/unit/about/preferencesTest.js

@@ -8,7 +8,7 @@ const {mount} = require('enzyme')
 const sinon = require('sinon')
 const assert = require('assert')
 const fakeElectron = require('../lib/fakeElectron')
-let Preferences
+let Preferences, appActions, SettingItemIcon
 require('../braveUnit')
 
 describe('Preferences component', function () {
@@ -46,11 +46,14 @@ describe('Preferences component', function () {
     mockery.registerMock('../../../../extensions/brave/img/coinbase_logo.png')
     mockery.registerMock('../../../../extensions/brave/img/android_download.svg')
     mockery.registerMock('../../../../extensions/brave/img/ios_download.svg')
+    mockery.registerMock('../../img/icon_pencil.svg')
 
     window.chrome = fakeElectron
     window.CustomEvent = {}
 
     Preferences = require('../../../js/about/preferences').AboutPreferences
+    SettingItemIcon = require('../../../app/renderer/components/settings').SettingItemIcon
+    appActions = require('../../../js/actions/appActions')
   })
   after(function () {
     mockery.disable()
@@ -90,4 +93,21 @@ describe('Preferences component', function () {
       assert.equal(this.result.find('[data-l10n-id="searchSettings"]').length, 1)
     })
   })
+
+  describe('General', function () {
+    describe('Default path', function () {
+      it('call appActions.defaultDownloadPath when pencil is clicked', function () {
+        const spy = sinon.spy(appActions, 'defaultDownloadPath')
+        const wrapper = mount(
+          <SettingItemIcon
+            clickAction={appActions.defaultDownloadPath()}
+            position='right'
+          />
+        )
+        wrapper.find('span[data-icon-position="right"]').simulate('click')
+        assert.equal(spy.calledOnce, true)
+        appActions.defaultDownloadPath.restore()
+      })
+    })
+  })
 })

test/unit/app/browser/reducers/downloadsReducerTest.js

@@ -231,4 +231,34 @@ describe('downloadsReducer', function () {
       })
     })
   })
+
+  describe('APP_DOWNLOAD_DEFAULT_PATH', function () {
+    describe('when showing folder selection dialog', function () {
+      let stub
+      let options
+
+      before(function () {
+        stub = sinon.stub(fakeElectron.dialog, 'showOpenDialog', function (arg1, arg2) {
+          options = arg2
+        })
+        downloadsReducer({}, {actionType: appConstants.APP_DOWNLOAD_DEFAULT_PATH})
+      })
+
+      after(function () {
+        stub.restore()
+      })
+
+      it('calls dialog.showOpenDialog', function () {
+        assert(stub.calledOnce)
+      })
+
+      it('passes the correct defaultPath', function () {
+        assert.equal(options.defaultPath, `${process.cwd()}/downloads`)
+      })
+
+      it('passes the correct properties object', function () {
+        assert.deepEqual(options.properties, ['openDirectory'])
+      })
+    })
+  })
 })

test/unit/lib/fakeElectron.js

@@ -27,12 +27,16 @@ const fakeElectron = {
   },
   app: {
     on: function () {
-    }
+    },
+    getPath: (param) => `${process.cwd()}/${param}`
   },
   clipboard: {
     writeText: function () {
     }
   },
+  dialog: {
+    showOpenDialog: function () { }
+  },
   shell: {
     showItemInFolder: function () {
     },