From ca2d74e1185626f6c9179bf0cf7382e010a49bf0 Mon Sep 17 00:00:00 2001 From: Heilig Benedek Date: Thu, 8 Nov 2018 15:51:06 +0100 Subject: [PATCH] feat: add methods to allow customization of save dialog during will-download event (#15497) * feat: add method to DownloadItem that allows customization of dialog options * docs: add docs for get/setSaveDialogOptions * add missing copy constructor for DialogSettings on mac and linux * fix: don't filter dialog options for mas build, don't return properties * test: add test for get/setSaveDialogOptions * fix: remove openDevtools added for debugging * test: fix failing test because of new event parameter * docs: use SaveDialogOptions instead of Object --- atom/browser/api/atom_api_dialog.cc | 45 +---------- atom/browser/api/atom_api_download_item.cc | 12 +++ atom/browser/api/atom_api_download_item.h | 4 + .../browser/atom_download_manager_delegate.cc | 22 +++++- atom/browser/atom_download_manager_delegate.h | 3 + atom/browser/ui/file_dialog_gtk.cc | 1 + atom/browser/ui/file_dialog_mac.mm | 1 + .../file_dialog_converter.cc | 76 +++++++++++++++++++ .../file_dialog_converter.h | 33 ++++++++ docs/api/download-item.md | 13 ++++ filenames.gni | 2 + spec/api-session-spec.js | 36 ++++++++- spec/static/main.js | 4 +- 13 files changed, 203 insertions(+), 49 deletions(-) create mode 100644 atom/common/native_mate_converters/file_dialog_converter.cc create mode 100644 atom/common/native_mate_converters/file_dialog_converter.h diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc index bdbaffbb5bd3..623f69ae27c4 100644 --- a/atom/browser/api/atom_api_dialog.cc +++ b/atom/browser/api/atom_api_dialog.cc @@ -12,6 +12,7 @@ #include "atom/browser/ui/file_dialog.h" #include "atom/browser/ui/message_box.h" #include "atom/common/native_mate_converters/callback.h" +#include "atom/common/native_mate_converters/file_dialog_converter.h" #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/native_mate_converters/image_converter.h" #include "atom/common/native_mate_converters/net_converter.h" @@ -19,50 +20,6 @@ #include "atom/common/node_includes.h" -namespace mate { - -template <> -struct Converter { - static bool FromV8(v8::Isolate* isolate, - v8::Local val, - file_dialog::Filter* out) { - mate::Dictionary dict; - if (!ConvertFromV8(isolate, val, &dict)) - return false; - if (!dict.Get("name", &(out->first))) - return false; - if (!dict.Get("extensions", &(out->second))) - return false; - return true; - } -}; - -template <> -struct Converter { - static bool FromV8(v8::Isolate* isolate, - v8::Local val, - file_dialog::DialogSettings* out) { - mate::Dictionary dict; - if (!ConvertFromV8(isolate, val, &dict)) - return false; - dict.Get("window", &(out->parent_window)); - dict.Get("title", &(out->title)); - dict.Get("message", &(out->message)); - dict.Get("buttonLabel", &(out->button_label)); - dict.Get("nameFieldLabel", &(out->name_field_label)); - dict.Get("defaultPath", &(out->default_path)); - dict.Get("filters", &(out->filters)); - dict.Get("properties", &(out->properties)); - dict.Get("showsTagField", &(out->shows_tag_field)); -#if defined(MAS_BUILD) - dict.Get("securityScopedBookmarks", &(out->security_scoped_bookmarks)); -#endif - return true; - } -}; - -} // namespace mate - namespace { void ShowMessageBox(int type, diff --git a/atom/browser/api/atom_api_download_item.cc b/atom/browser/api/atom_api_download_item.cc index 4fa4d5517b4a..67b2ae05ce1a 100644 --- a/atom/browser/api/atom_api_download_item.cc +++ b/atom/browser/api/atom_api_download_item.cc @@ -8,6 +8,7 @@ #include "atom/browser/atom_browser_main_parts.h" #include "atom/common/native_mate_converters/callback.h" +#include "atom/common/native_mate_converters/file_dialog_converter.h" #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/native_mate_converters/gurl_converter.h" #include "base/strings/utf_string_conversions.h" @@ -165,6 +166,15 @@ base::FilePath DownloadItem::GetSavePath() const { return save_path_; } +file_dialog::DialogSettings DownloadItem::GetSaveDialogOptions() const { + return dialog_options_; +} + +void DownloadItem::SetSaveDialogOptions( + const file_dialog::DialogSettings& options) { + dialog_options_ = options; +} + std::string DownloadItem::GetLastModifiedTime() const { return download_item_->GetLastModifiedTime(); } @@ -200,6 +210,8 @@ void DownloadItem::BuildPrototype(v8::Isolate* isolate, .SetMethod("isDone", &DownloadItem::IsDone) .SetMethod("setSavePath", &DownloadItem::SetSavePath) .SetMethod("getSavePath", &DownloadItem::GetSavePath) + .SetMethod("setSaveDialogOptions", &DownloadItem::SetSaveDialogOptions) + .SetMethod("getSaveDialogOptions", &DownloadItem::GetSaveDialogOptions) .SetMethod("getLastModifiedTime", &DownloadItem::GetLastModifiedTime) .SetMethod("getETag", &DownloadItem::GetETag) .SetMethod("getStartTime", &DownloadItem::GetStartTime); diff --git a/atom/browser/api/atom_api_download_item.h b/atom/browser/api/atom_api_download_item.h index 45ed54faf191..9ab3050d94b9 100644 --- a/atom/browser/api/atom_api_download_item.h +++ b/atom/browser/api/atom_api_download_item.h @@ -9,6 +9,7 @@ #include #include "atom/browser/api/trackable_object.h" +#include "atom/browser/ui/file_dialog.h" #include "base/files/file_path.h" #include "components/download/public/common/download_item.h" #include "native_mate/handle.h" @@ -44,6 +45,8 @@ class DownloadItem : public mate::TrackableObject, bool IsDone() const; void SetSavePath(const base::FilePath& path); base::FilePath GetSavePath() const; + file_dialog::DialogSettings GetSaveDialogOptions() const; + void SetSaveDialogOptions(const file_dialog::DialogSettings& options); std::string GetLastModifiedTime() const; std::string GetETag() const; double GetStartTime() const; @@ -58,6 +61,7 @@ class DownloadItem : public mate::TrackableObject, private: base::FilePath save_path_; + file_dialog::DialogSettings dialog_options_; download::DownloadItem* download_item_; DISALLOW_COPY_AND_ASSIGN(DownloadItem); diff --git a/atom/browser/atom_download_manager_delegate.cc b/atom/browser/atom_download_manager_delegate.cc index 112fa7be8cf2..8940640750ae 100644 --- a/atom/browser/atom_download_manager_delegate.cc +++ b/atom/browser/atom_download_manager_delegate.cc @@ -70,6 +70,18 @@ void AtomDownloadManagerDelegate::GetItemSavePath(download::DownloadItem* item, *path = download->GetSavePath(); } +void AtomDownloadManagerDelegate::GetItemSaveDialogOptions( + download::DownloadItem* item, + file_dialog::DialogSettings* options) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + v8::Locker locker(isolate); + v8::HandleScope handle_scope(isolate); + api::DownloadItem* download = + api::DownloadItem::FromWrappedClass(isolate, item); + if (download) + *options = download->GetSaveDialogOptions(); +} + void AtomDownloadManagerDelegate::OnDownloadPathGenerated( uint32_t download_id, const content::DownloadTargetCallback& callback, @@ -96,10 +108,14 @@ void AtomDownloadManagerDelegate::OnDownloadPathGenerated( GetItemSavePath(item, &path); // Show save dialog if save path was not set already on item file_dialog::DialogSettings settings; - settings.parent_window = window; + GetItemSaveDialogOptions(item, &settings); + if (!settings.parent_window) + settings.parent_window = window; settings.force_detached = offscreen; - settings.title = item->GetURL().spec(); - settings.default_path = default_path; + if (settings.title.size() == 0) + settings.title = item->GetURL().spec(); + if (!settings.default_path.empty()) + settings.default_path = default_path; if (path.empty() && file_dialog::ShowSaveDialog(settings, &path)) { // Remember the last selected download directory. AtomBrowserContext* browser_context = static_cast( diff --git a/atom/browser/atom_download_manager_delegate.h b/atom/browser/atom_download_manager_delegate.h index f1cc1190d4bf..a373b5e2bc55 100644 --- a/atom/browser/atom_download_manager_delegate.h +++ b/atom/browser/atom_download_manager_delegate.h @@ -7,6 +7,7 @@ #include +#include "atom/browser/ui/file_dialog.h" #include "base/memory/weak_ptr.h" #include "content/public/browser/download_manager_delegate.h" @@ -41,6 +42,8 @@ class AtomDownloadManagerDelegate : public content::DownloadManagerDelegate { private: // Get the save path set on the associated api::DownloadItem object void GetItemSavePath(download::DownloadItem* item, base::FilePath* path); + void GetItemSaveDialogOptions(download::DownloadItem* item, + file_dialog::DialogSettings* settings); content::DownloadManager* download_manager_; base::WeakPtrFactory weak_ptr_factory_; diff --git a/atom/browser/ui/file_dialog_gtk.cc b/atom/browser/ui/file_dialog_gtk.cc index cd9c496e65b5..b3f29ba9cb0c 100644 --- a/atom/browser/ui/file_dialog_gtk.cc +++ b/atom/browser/ui/file_dialog_gtk.cc @@ -18,6 +18,7 @@ namespace file_dialog { DialogSettings::DialogSettings() = default; +DialogSettings::DialogSettings(const DialogSettings&) = default; DialogSettings::~DialogSettings() = default; namespace { diff --git a/atom/browser/ui/file_dialog_mac.mm b/atom/browser/ui/file_dialog_mac.mm index 18b18a8b240c..231c006d346e 100644 --- a/atom/browser/ui/file_dialog_mac.mm +++ b/atom/browser/ui/file_dialog_mac.mm @@ -74,6 +74,7 @@ namespace file_dialog { DialogSettings::DialogSettings() = default; +DialogSettings::DialogSettings(const DialogSettings&) = default; DialogSettings::~DialogSettings() = default; namespace { diff --git a/atom/common/native_mate_converters/file_dialog_converter.cc b/atom/common/native_mate_converters/file_dialog_converter.cc new file mode 100644 index 000000000000..ff2567e67f25 --- /dev/null +++ b/atom/common/native_mate_converters/file_dialog_converter.cc @@ -0,0 +1,76 @@ +// Copyright (c) 2015 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/common/native_mate_converters/file_dialog_converter.h" + +#include "atom/browser/api/atom_api_browser_window.h" +#include "atom/browser/ui/file_dialog.h" +#include "atom/common/native_mate_converters/file_path_converter.h" +#include "native_mate/dictionary.h" + +namespace mate { + +bool Converter::FromV8(v8::Isolate* isolate, + v8::Local val, + file_dialog::Filter* out) { + mate::Dictionary dict; + if (!ConvertFromV8(isolate, val, &dict)) + return false; + if (!dict.Get("name", &(out->first))) + return false; + if (!dict.Get("extensions", &(out->second))) + return false; + return true; +} + +v8::Local Converter::ToV8( + v8::Isolate* isolate, + const file_dialog::Filter& in) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate); + + dict.Set("name", in.first); + dict.Set("extensions", in.second); + + return dict.GetHandle(); +} + +bool Converter::FromV8( + v8::Isolate* isolate, + v8::Local val, + file_dialog::DialogSettings* out) { + mate::Dictionary dict; + if (!ConvertFromV8(isolate, val, &dict)) + return false; + dict.Get("window", &(out->parent_window)); + dict.Get("title", &(out->title)); + dict.Get("message", &(out->message)); + dict.Get("buttonLabel", &(out->button_label)); + dict.Get("nameFieldLabel", &(out->name_field_label)); + dict.Get("defaultPath", &(out->default_path)); + dict.Get("filters", &(out->filters)); + dict.Get("properties", &(out->properties)); + dict.Get("showsTagField", &(out->shows_tag_field)); + dict.Get("securityScopedBookmarks", &(out->security_scoped_bookmarks)); + return true; +} + +v8::Local Converter::ToV8( + v8::Isolate* isolate, + const file_dialog::DialogSettings& in) { + mate::Dictionary dict = mate::Dictionary::CreateEmpty(isolate); + + dict.Set("window", atom::api::BrowserWindow::From(isolate, in.parent_window)); + dict.Set("title", in.title); + dict.Set("message", in.message); + dict.Set("buttonLabel", in.button_label); + dict.Set("nameFieldLabel", in.name_field_label); + dict.Set("defaultPath", in.default_path); + dict.Set("filters", in.filters); + dict.Set("showsTagField", in.shows_tag_field); + dict.Set("securityScopedBookmarks", in.security_scoped_bookmarks); + + return dict.GetHandle(); +} + +} // namespace mate diff --git a/atom/common/native_mate_converters/file_dialog_converter.h b/atom/common/native_mate_converters/file_dialog_converter.h new file mode 100644 index 000000000000..7d4b5f1217de --- /dev/null +++ b/atom/common/native_mate_converters/file_dialog_converter.h @@ -0,0 +1,33 @@ +// Copyright (c) 2014 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_COMMON_NATIVE_MATE_CONVERTERS_FILE_DIALOG_CONVERTER_H_ +#define ATOM_COMMON_NATIVE_MATE_CONVERTERS_FILE_DIALOG_CONVERTER_H_ + +#include "atom/browser/ui/file_dialog.h" +#include "native_mate/converter.h" + +namespace mate { + +template <> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const file_dialog::Filter& in); + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + file_dialog::Filter* out); +}; + +template <> +struct Converter { + static v8::Local ToV8(v8::Isolate* isolate, + const file_dialog::DialogSettings& in); + static bool FromV8(v8::Isolate* isolate, + v8::Local val, + file_dialog::DialogSettings* out); +}; + +} // namespace mate + +#endif // ATOM_COMMON_NATIVE_MATE_CONVERTERS_FILE_DIALOG_CONVERTER_H_ diff --git a/docs/api/download-item.md b/docs/api/download-item.md index dcf058a4bc17..e3c6c393f89b 100644 --- a/docs/api/download-item.md +++ b/docs/api/download-item.md @@ -88,6 +88,19 @@ Returns `String` - The save path of the download item. This will be either the p set via `downloadItem.setSavePath(path)` or the path selected from the shown save dialog. +#### `downloadItem.setSaveDialogOptions(options)` + +* `options` SaveDialogOptions - Set the save file dialog options. This object has the same +properties as the `options` parameter of [`dialog.showSaveDialog()`](dialog.md). + +This API allows the user to set custom options for the save dialog that opens +for the download item by default. +The API is only available in session's `will-download` callback function. + +#### `downloadItem.getSaveDialogOptions()` + +Returns `SaveDialogOptions` - Returns the object previously set by `downloadItem.setSaveDialogOptions(options)`. + #### `downloadItem.pause()` Pauses the download. diff --git a/filenames.gni b/filenames.gni index 7ce785819ed9..2ef3f0e8843c 100644 --- a/filenames.gni +++ b/filenames.gni @@ -595,6 +595,8 @@ filenames = { "atom/common/native_mate_converters/callback.h", "atom/common/native_mate_converters/content_converter.cc", "atom/common/native_mate_converters/content_converter.h", + "atom/common/native_mate_converters/file_dialog_converter.cc", + "atom/common/native_mate_converters/file_dialog_converter.h", "atom/common/native_mate_converters/file_path_converter.h", "atom/common/native_mate_converters/gfx_converter.cc", "atom/common/native_mate_converters/gfx_converter.h", diff --git a/spec/api-session-spec.js b/spec/api-session-spec.js index 1b9520051ce9..a0f0066642a9 100644 --- a/spec/api-session-spec.js +++ b/spec/api-session-spec.js @@ -1,4 +1,5 @@ const assert = require('assert') +const chai = require('chai') const http = require('http') const https = require('https') const path = require('path') @@ -9,6 +10,7 @@ const { closeWindow } = require('./window-helpers') const { ipcRenderer, remote } = require('electron') const { ipcMain, session, BrowserWindow, net } = remote +const { expect } = chai /* The whole session API doesn't use standard callbacks */ /* eslint-disable standard/no-callback-literal */ @@ -421,6 +423,38 @@ describe('session module', () => { }) }) + it('can set options for the save dialog', (done) => { + downloadServer.listen(0, '127.0.0.1', () => { + const filePath = path.join(__dirname, 'fixtures', 'mock.pdf') + const port = downloadServer.address().port + const options = { + window: null, + title: 'title', + message: 'message', + buttonLabel: 'buttonLabel', + nameFieldLabel: 'nameFieldLabel', + defaultPath: '/', + filters: [{ + name: '1', extensions: ['.1', '.2'] + }, { + name: '2', extensions: ['.3', '.4', '.5'] + }], + showsTagField: true, + securityScopedBookmarks: true + } + + ipcRenderer.sendSync('set-download-option', true, false, filePath, options) + w.webContents.downloadURL(`${url}:${port}`) + ipcRenderer.once('download-done', (event, state, url, + mimeType, receivedBytes, + totalBytes, disposition, + filename, savePath, dialogOptions) => { + expect(dialogOptions).to.deep.equal(options) + done() + }) + }) + }) + describe('when a save path is specified and the URL is unavailable', () => { it('does not display a save dialog and reports the done state as interrupted', (done) => { ipcRenderer.sendSync('set-download-option', false, false) @@ -697,7 +731,7 @@ describe('session module', () => { const downloadUrl = `http://127.0.0.1:${port}/assets/logo.png` const callback = (event, state, url, mimeType, receivedBytes, totalBytes, disposition, - filename, savePath, urlChain, + filename, savePath, dialogOptions, urlChain, lastModifiedTime, eTag) => { if (state === 'cancelled') { const options = { diff --git a/spec/static/main.js b/spec/static/main.js index 415d96b0c57f..3de47a48a611 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -161,7 +161,7 @@ app.on('ready', function () { // For session's download test, listen 'will-download' event in browser, and // reply the result to renderer for verifying const downloadFilePath = path.join(__dirname, '..', 'fixtures', 'mock.pdf') - ipcMain.on('set-download-option', function (event, needCancel, preventDefault, filePath = downloadFilePath) { + ipcMain.on('set-download-option', function (event, needCancel, preventDefault, filePath = downloadFilePath, dialogOptions = {}) { window.webContents.session.once('will-download', function (e, item) { window.webContents.send('download-created', item.getState(), @@ -187,6 +187,7 @@ app.on('ready', function () { item.resume() } else { item.setSavePath(filePath) + item.setSaveDialogOptions(dialogOptions) } item.on('done', function (e, state) { window.webContents.send('download-done', @@ -198,6 +199,7 @@ app.on('ready', function () { item.getContentDisposition(), item.getFilename(), item.getSavePath(), + item.getSaveDialogOptions(), item.getURLChain(), item.getLastModifiedTime(), item.getETag())