feat: add methods to allow customization of save dialog during will-d…

…ownload 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
electron · Nov 8, 2018 · ca2d74e · ca2d74e
1 parent 673004b
commit ca2d74e
Show file tree

Hide file tree

Showing 13 changed files with 203 additions and 49 deletions.
diff --git a/atom/browser/api/atom_api_dialog.cc b/atom/browser/api/atom_api_dialog.cc
@@ -12,57 +12,14 @@
 #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"
 #include "native_mate/dictionary.h"
 
 #include "atom/common/node_includes.h"
 
-namespace mate {
-
-template <>
-struct Converter<file_dialog::Filter> {
-  static bool FromV8(v8::Isolate* isolate,
-                     v8::Local<v8::Value> 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<file_dialog::DialogSettings> {
-  static bool FromV8(v8::Isolate* isolate,
-                     v8::Local<v8::Value> 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
@@ -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
@@ -9,6 +9,7 @@
 #include <vector>
 
 #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<DownloadItem>,
   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<DownloadItem>,
 
  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
@@ -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<AtomBrowserContext*>(

diff --git a/atom/browser/atom_download_manager_delegate.h b/atom/browser/atom_download_manager_delegate.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#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<AtomDownloadManagerDelegate> weak_ptr_factory_;

diff --git 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
@@ -74,6 +74,7 @@ - (void)dealloc {
 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
@@ -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<file_dialog::Filter>::FromV8(v8::Isolate* isolate,
+                                            v8::Local<v8::Value> 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<v8::Value> Converter<file_dialog::Filter>::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<file_dialog::DialogSettings>::FromV8(
+    v8::Isolate* isolate,
+    v8::Local<v8::Value> 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<v8::Value> Converter<file_dialog::DialogSettings>::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
@@ -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<file_dialog::Filter> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const file_dialog::Filter& in);
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     file_dialog::Filter* out);
+};
+
+template <>
+struct Converter<file_dialog::DialogSettings> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const file_dialog::DialogSettings& in);
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> 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
@@ -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
@@ -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
@@ -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 = {