Skip to content
This repository was archived by the owner on Dec 11, 2019. It is now read-only.

Commit 2f9ef72

Browse files
bscliftonbsclifton
authored
Merge pull request #9867 from timborden/feature/download-filename-exists
Don't overwrite existing files when DOWNLOAD_ALWAYS_ASK is false
2 parents f3d5566 + 3cba178 commit 2f9ef72

File tree

3 files changed

+133
-64
lines changed

3 files changed

+133
-64
lines changed

‎app/filtering.js

+17-2
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const dialog = electron.dialog
2727
const app = electron.app
2828
const uuid = require('uuid')
2929
const path = require('path')
30+
const fs = require('fs')
3031
const getOrigin = require('../js/lib/urlutil').getOrigin
3132
const {adBlockResourceName} = require('./adBlock')
3233
const {updateElectronDownloadItem} = require('./browser/electronDownloadItem')
@@ -541,8 +542,22 @@ function registerForDownloadListener (session) {
541542
itemFilename = item.getFilename()
542543
}
543544

544-
const defaultPath = path.join(getSetting(settings.DOWNLOAD_DEFAULT_PATH) || getSetting(settings.DEFAULT_DOWNLOAD_SAVE_PATH) || app.getPath('downloads'), itemFilename)
545-
const savePath = ((process.env.SPECTRON || (!getSetting(settings.DOWNLOAD_ALWAYS_ASK) && !item.promptForSaveLocation())) ? defaultPath : dialog.showSaveDialog(win, { defaultPath }))
545+
const defaultDir = (getSetting(settings.DOWNLOAD_DEFAULT_PATH) || getSetting(settings.DEFAULT_DOWNLOAD_SAVE_PATH) || app.getPath('downloads'))
546+
let savePath
547+
if (process.env.SPECTRON || (!getSetting(settings.DOWNLOAD_ALWAYS_ASK) && !item.promptForSaveLocation())) {
548+
let willOverwrite = true
549+
let matchedFilenames = 0
550+
while (willOverwrite) {
551+
savePath = path.join(defaultDir, (matchedFilenames ? `${itemFilename.replace(new RegExp(`${path.extname(itemFilename)}$`), '')} (${matchedFilenames})${path.extname(itemFilename)}` : itemFilename))
552+
if (!fs.existsSync(savePath)) {
553+
willOverwrite = false
554+
} else {
555+
matchedFilenames++
556+
}
557+
}
558+
} else {
559+
savePath = dialog.showSaveDialog(win, { defaultPath: path.join(defaultDir, itemFilename) })
560+
}
546561

547562
// User cancelled out of save dialog prompt
548563
if (!savePath) {

‎test/lib/selectors.js

+1
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ module.exports = {
123123
downloadPause: '[data-test-id="pauseButton"]',
124124
downloadResume: '[data-test-id="resumeButton"]',
125125
downloadCancel: '[data-test-id="cancelButton"]',
126+
downloadComplete: '.downloadItem.completed',
126127
downloadReDownload: '[data-test-id="redownloadButton"]',
127128
downloadDelete: '[data-test-id="deleteButton"]',
128129
downloadDeleteConfirm: '[data-test-id="confirmDeleteButton"]',

‎test/misc-components/downloadItemTest.js

+115-62
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
/* This Source Code Form is subject to the terms of the Mozilla Public
22
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
33
* You can obtain one at http://mozilla.org/MPL/2.0/. */
4-
/* global describe, it, before */
4+
/* global describe, it, before, after */
55

6+
const os = require('os')
7+
const path = require('path')
8+
const fs = require('fs-extra')
69
const Brave = require('../lib/brave')
710
const {
811
urlInput,
@@ -11,11 +14,13 @@ const {
1114
downloadPause,
1215
downloadResume,
1316
downloadCancel,
17+
downloadComplete,
1418
downloadReDownload,
1519
downloadRemoveFromList,
1620
downloadDelete,
1721
downloadDeleteConfirm
1822
} = require('../lib/selectors')
23+
const settingsConst = require('../../js/constants/settings')
1924

2025
function * setup (client) {
2126
yield client
@@ -24,75 +29,123 @@ function * setup (client) {
2429
.waitForVisible(urlInput)
2530
}
2631

27-
describe('downloadItem test', function () {
28-
Brave.beforeAll(this)
29-
before(function * () {
30-
this.downloadSite = 'http://releases.ubuntu.com/16.04.2/ubuntu-16.04.2-desktop-amd64.iso'
31-
yield setup(this.app.client)
32+
describe('Downloads', function () {
33+
describe('Location and file naming tests', function () {
34+
let tempDownloadPath
35+
Brave.beforeAll(this)
36+
before(function * () {
37+
tempDownloadPath = path.join(os.tmpdir(), 'brave-test', 'downloads')
38+
this.downloadFile = 'Brave_proudly-partner_badges.zip'
39+
this.downloadSite = `https://brave.com/about/${this.downloadFile}`
40+
this.renamedFile = this.downloadFile.slice(0, this.downloadFile.indexOf('.')) + ' (1)' + this.downloadFile.slice(this.downloadFile.indexOf('.'))
3241

33-
yield this.app.client
34-
.waitForUrl(Brave.newTabUrl)
35-
.url(this.downloadSite)
36-
})
42+
yield setup(this.app.client)
3743

38-
it('check if download bar is shown', function * () {
39-
yield this.app.client
40-
.windowByUrl(Brave.browserWindowUrl)
41-
.waitForElementCount(downloadBar, 1)
42-
})
44+
yield this.app.client
45+
.waitForUrl(Brave.newTabUrl)
46+
})
4347

44-
it('check if you can pause download', function * () {
45-
yield this.app.client
46-
.moveToObject(downloadItem)
47-
.waitForElementCount(downloadPause, 1)
48-
.click(downloadPause)
49-
.waitForElementCount(downloadResume, 1)
50-
})
48+
after(function * () {
49+
yield new Promise((resolve, reject) => {
50+
return fs.remove(tempDownloadPath, (err) => err ? reject(err) : resolve())
51+
})
52+
})
5153

52-
it('check if you can resume download', function * () {
53-
yield this.app.client
54-
.waitForElementCount(downloadResume, 1)
55-
.click(downloadResume)
56-
.waitForElementCount(downloadPause, 1)
57-
})
54+
it('check if first download completes', function * () {
55+
yield this.app.client
56+
.windowByUrl(Brave.browserWindowUrl)
57+
.changeSetting(settingsConst.DOWNLOAD_DEFAULT_PATH, tempDownloadPath)
58+
.url(this.downloadSite)
59+
.waitForElementCount(downloadComplete, 1)
5860

59-
it('check if you can cancel download', function * () {
60-
yield this.app.client
61-
.waitForElementCount(downloadPause, 1)
62-
.click(downloadCancel)
63-
.waitForElementCount(downloadReDownload, 1)
64-
})
61+
yield new Promise((resolve, reject) => {
62+
return fs.exists(path.join(tempDownloadPath, this.downloadFile), (res) => res ? resolve(res) : reject(res))
63+
}).should.eventually.be.true
64+
})
6565

66-
it('check if you can re-download', function * () {
67-
yield this.app.client
68-
.waitForElementCount(downloadReDownload, 1)
69-
.click(downloadReDownload)
70-
.waitForElementCount(downloadPause, 1)
71-
})
66+
it('check if second download completes and is renamed', function * () {
67+
yield this.app.client
68+
.windowByUrl(Brave.browserWindowUrl)
69+
.changeSetting(settingsConst.DOWNLOAD_DEFAULT_PATH, tempDownloadPath)
70+
.url(this.downloadSite)
71+
.waitForElementCount(downloadComplete, 2)
7272

73-
it('check if you can remove item from the list', function * () {
74-
yield this.app.client
75-
.moveToObject(downloadItem)
76-
.waitForElementCount(downloadPause, 1)
77-
.click(downloadCancel)
78-
.waitForElementCount(downloadReDownload, 1)
79-
.click(downloadRemoveFromList)
80-
.waitForElementCount(downloadBar, 0)
73+
yield new Promise((resolve, reject) => {
74+
return fs.exists(path.join(tempDownloadPath, this.renamedFile), (res) => res ? resolve(res) : reject(res))
75+
}).should.eventually.be.true
76+
})
8177
})
8278

83-
it('check if you can delete downloaded item', function * () {
84-
yield this.app.client
85-
.tabByIndex(0)
86-
.url(this.downloadSite)
87-
.windowByUrl(Brave.browserWindowUrl)
88-
.waitForElementCount(downloadBar, 1)
89-
.moveToObject(downloadItem)
90-
.waitForElementCount(downloadPause, 1)
91-
.click(downloadCancel)
92-
.waitForElementCount(downloadReDownload, 1)
93-
.click(downloadDelete)
94-
.waitForElementCount(downloadDeleteConfirm, 1)
95-
.click(downloadDeleteConfirm)
96-
.waitForElementCount(downloadBar, 0)
79+
describe('Item and bar tests', function () {
80+
Brave.beforeAll(this)
81+
before(function * () {
82+
this.downloadSite = 'http://releases.ubuntu.com/16.04.2/ubuntu-16.04.2-desktop-amd64.iso'
83+
yield setup(this.app.client)
84+
85+
yield this.app.client
86+
.waitForUrl(Brave.newTabUrl)
87+
.url(this.downloadSite)
88+
})
89+
90+
it('check if download bar is shown', function * () {
91+
yield this.app.client
92+
.windowByUrl(Brave.browserWindowUrl)
93+
.waitForElementCount(downloadBar, 1)
94+
})
95+
96+
it('check if you can pause download', function * () {
97+
yield this.app.client
98+
.moveToObject(downloadItem)
99+
.waitForElementCount(downloadPause, 1)
100+
.click(downloadPause)
101+
.waitForElementCount(downloadResume, 1)
102+
})
103+
104+
it('check if you can resume download', function * () {
105+
yield this.app.client
106+
.waitForElementCount(downloadResume, 1)
107+
.click(downloadResume)
108+
.waitForElementCount(downloadPause, 1)
109+
})
110+
111+
it('check if you can cancel download', function * () {
112+
yield this.app.client
113+
.waitForElementCount(downloadPause, 1)
114+
.click(downloadCancel)
115+
.waitForElementCount(downloadReDownload, 1)
116+
})
117+
118+
it('check if you can re-download', function * () {
119+
yield this.app.client
120+
.waitForElementCount(downloadReDownload, 1)
121+
.click(downloadReDownload)
122+
.waitForElementCount(downloadPause, 1)
123+
})
124+
125+
it('check if you can remove item from the list', function * () {
126+
yield this.app.client
127+
.moveToObject(downloadItem)
128+
.waitForElementCount(downloadPause, 1)
129+
.click(downloadCancel)
130+
.waitForElementCount(downloadReDownload, 1)
131+
.click(downloadRemoveFromList)
132+
.waitForElementCount(downloadBar, 0)
133+
})
134+
135+
it('check if you can delete downloaded item', function * () {
136+
yield this.app.client
137+
.tabByIndex(0)
138+
.url(this.downloadSite)
139+
.windowByUrl(Brave.browserWindowUrl)
140+
.waitForElementCount(downloadBar, 1)
141+
.moveToObject(downloadItem)
142+
.waitForElementCount(downloadPause, 1)
143+
.click(downloadCancel)
144+
.waitForElementCount(downloadReDownload, 1)
145+
.click(downloadDelete)
146+
.waitForElementCount(downloadDeleteConfirm, 1)
147+
.click(downloadDeleteConfirm)
148+
.waitForElementCount(downloadBar, 0)
149+
})
97150
})
98151
})

0 commit comments

Comments
 (0)