diff --git a/app/browser/reducers/downloadsReducer.js b/app/browser/reducers/downloadsReducer.js index 02547ab1200..cca41ad8b00 100644 --- a/app/browser/reducers/downloadsReducer.js +++ b/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) { @@ -68,7 +71,7 @@ const downloadsReducer = (state, action) => { case appConstants.APP_DOWNLOAD_REDOWNLOADED: const win = BrowserWindow.getFocusedWindow() if (win) { - win.webContents.downloadURL(download.get('url')) + win.webContents.downloadURL(download.get('url'), true) state = state.deleteIn(['downloads', action.downloadId]) } else { shell.beep() @@ -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 } diff --git a/app/extensions/brave/locales/en-US/preferences.properties b/app/extensions/brave/locales/en-US/preferences.properties index de1baa2c391..2a8724229ea 100644 --- a/app/extensions/brave/locales/en-US/preferences.properties +++ b/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 diff --git a/app/filtering.js b/app/filtering.js index 02418daff2f..c604c1667f4 100644 --- a/app/filtering.js +++ b/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) && !item.promptForSaveLocation())) ? defaultPath : dialog.showSaveDialog(win, { defaultPath })) // User cancelled out of save dialog prompt if (!savePath) { diff --git a/app/ledger.js b/app/ledger.js index 0fede6932c3..2b28394170d 100644 --- a/app/ledger.js +++ b/app/ledger.js @@ -323,7 +323,7 @@ var backupKeys = (appState, action) => { if (action.backupAction === 'print') { webContents.print({silent: false, printBackground: false}) } else { - webContents.downloadURL(fileUrl(filePath)) + webContents.downloadURL(fileUrl(filePath), true) } }) } diff --git a/app/pdf.js b/app/pdf.js index 81fc5b5a3c8..43a70382519 100644 --- a/app/pdf.js +++ b/app/pdf.js @@ -32,7 +32,7 @@ const renderUrlToPdf = (appState, action, testingMode) => { let listeners = wv.session.listeners('will-download') wv.session.removeAllListeners('will-download') - wv.downloadURL(pdfDataURI) + wv.downloadURL(pdfDataURI, true) wv.session.once('will-download', function (event, item) { if (savePath) { item.setSavePath(savePath) diff --git a/app/renderer/components/settings.js b/app/renderer/components/settings.js index 07cd01ae023..f2523d7f5ec 100644 --- a/app/renderer/components/settings.js +++ b/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
+ { + this.props.dataL10nId + ? + : null + } +
+ { + this.props.position === 'left' + ? + : null + } +
+ {this.props.children} +
+ { + this.props.position === 'right' + ? + : null + } +
+
+ } +} + +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 } diff --git a/docs/appActions.md b/docs/appActions.md index 1db6bd31460..7d18c235ebd 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -792,6 +792,12 @@ Action triggered by un-registering navigation handler +### defaultDownloadPath() + +Open dialog for default download path setting + + + * * * diff --git a/img/icon_pencil.svg b/img/icon_pencil.svg new file mode 100644 index 00000000000..db5ca30ec8c --- /dev/null +++ b/img/icon_pencil.svg @@ -0,0 +1,16 @@ + + + + + + + + + + diff --git a/js/about/preferences.js b/js/about/preferences.js index c531f12e80b..4d39837c747 100644 --- a/js/about/preferences.js +++ b/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 : } + + + + diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 2a442bc5edf..09d2268b3a7 100644 --- a/js/actions/appActions.js +++ b/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 + }) } } diff --git a/js/components/frame.js b/js/components/frame.js index 7aac451b5dd..861e16340fa 100644 --- a/js/components/frame.js +++ b/js/components/frame.js @@ -527,7 +527,7 @@ class Frame extends ImmutableComponent { ? UrlUtil.getLocationIfPDF(this.tab.get('url')) : this.tab.get('url') // TODO: Sometimes this tries to save in a non-existent directory - this.webview.downloadURL(downloadLocation) + this.webview.downloadURL(downloadLocation, true) break case 'print': this.webview.print() diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js index 0138d22f544..c80e098b2e1 100644 --- a/js/constants/appConfig.js +++ b/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, diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 05f99bdc4de..2e3785d1203 100644 --- a/js/constants/appConstants.js +++ b/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: _, diff --git a/js/constants/settings.js b/js/constants/settings.js index 6d28b66e4fc..f42ab8d5de8 100644 --- a/js/constants/settings.js +++ b/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', diff --git a/js/contextMenus.js b/js/contextMenus.js index 71dbc02632e..bf91690ef43 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -868,7 +868,7 @@ const saveAsMenuItem = (label, location) => { label: locale.translation(label), click: (item, focusedWindow) => { if (focusedWindow && location) { - focusedWindow.webContents.downloadURL(location) + focusedWindow.webContents.downloadURL(location, true) } } } diff --git a/test/unit/about/preferencesTest.js b/test/unit/about/preferencesTest.js index 6c1fa572843..5db6213fab7 100644 --- a/test/unit/about/preferencesTest.js +++ b/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( + + ) + wrapper.find('span[data-icon-position="right"]').simulate('click') + assert.equal(spy.calledOnce, true) + appActions.defaultDownloadPath.restore() + }) + }) + }) }) diff --git a/test/unit/app/browser/reducers/downloadsReducerTest.js b/test/unit/app/browser/reducers/downloadsReducerTest.js index dea890f5aa9..0162885ce44 100644 --- a/test/unit/app/browser/reducers/downloadsReducerTest.js +++ b/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']) + }) + }) + }) }) diff --git a/test/unit/lib/fakeElectron.js b/test/unit/lib/fakeElectron.js index 68d7f33b5b4..c5e562ef038 100644 --- a/test/unit/lib/fakeElectron.js +++ b/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 () { },