Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Use async save dialog for anchor download attribute (backport: 5-0-x) #16640

Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

fix: Use async save dialog for anchor download attribute

On macOS, the synchronous save dialog does not work properly and breaks
in weird ways when the dialog is expanded/collapsed (see #14606). This
works around the problem for the dialog shown for `<a download=...>` by
switching to the asynchronous API.

To test this:

main.js:
```
const { app, BrowserWindow, session } = require('electron')
const path = require('path')

function createWindow() {
  win = new BrowserWindow({
    width: 600,
    height: 600,
    webPreferences: {
      nodeIntegration: false,
    },
  })

  session.defaultSession.on('will-download', (event, item) => {
    // Test with and without this commented out.
    //item.setSavePath(path.join(app.getPath('desktop'), 'test-download.txt'))
  })

  win.loadURL(`file://${__dirname}/test.html`)
}

app.on('ready', createWindow)
```

test.html:
```
<html>
<head>
  <meta http-equiv="Content-Security-Policy" content="default-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline'; img-src 'self' data:">
</head>
<body>
<a href="data:application/xml;charset=utf-8,foo" download="download.txt">Save</a>
</body>
```
  • Loading branch information...
poiru authored and electron-bot committed Jan 30, 2019
commit e576e8713bbb7750a4edc228ada6fd7358f15729
@@ -100,23 +100,58 @@ void AtomDownloadManagerDelegate::OnDownloadPathGenerated(
if (relay)
window = relay->GetNativeWindow();

auto* web_preferences = WebContentsPreferences::From(web_contents);
bool offscreen =
!web_preferences || web_preferences->IsEnabled(options::kOffscreen);

// Show save dialog if save path was not set already on item
base::FilePath path;
GetItemSavePath(item, &path);
// Show save dialog if save path was not set already on item
file_dialog::DialogSettings settings;
GetItemSaveDialogOptions(item, &settings);
if (!settings.parent_window)
settings.parent_window = window;
settings.force_detached = offscreen;
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)) {
if (path.empty()) {
file_dialog::DialogSettings settings;
GetItemSaveDialogOptions(item, &settings);

if (!settings.parent_window)
settings.parent_window = window;
if (settings.title.size() == 0)
settings.title = item->GetURL().spec();
if (!settings.default_path.empty())
settings.default_path = default_path;

auto* web_preferences = WebContentsPreferences::From(web_contents);
const bool offscreen =
!web_preferences || web_preferences->IsEnabled(options::kOffscreen);
settings.force_detached = offscreen;

auto dialog_callback =
base::Bind(&AtomDownloadManagerDelegate::OnDownloadSaveDialogDone,
base::Unretained(this), download_id, callback);
file_dialog::ShowSaveDialog(settings, dialog_callback);
} else {
callback.Run(path, download::DownloadItem::TARGET_DISPOSITION_PROMPT,
download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, path,
download::DOWNLOAD_INTERRUPT_REASON_NONE);
};
}

#if defined(MAS_BUILD)
void AtomDownloadManagerDelegate::OnDownloadSaveDialogDone(
uint32_t download_id,
const content::DownloadTargetCallback& download_callback,
bool result,
const base::FilePath& path,
const std::string& bookmark)
#else
void AtomDownloadManagerDelegate::OnDownloadSaveDialogDone(
uint32_t download_id,
const content::DownloadTargetCallback& download_callback,
bool result,
const base::FilePath& path)
#endif
{
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

auto* item = download_manager_->GetDownload(download_id);
if (!item)
return;

if (result) {
// Remember the last selected download directory.
AtomBrowserContext* browser_context = static_cast<AtomBrowserContext*>(
download_manager_->GetBrowserContext());
@@ -133,12 +168,15 @@ void AtomDownloadManagerDelegate::OnDownloadPathGenerated(
}

// Running the DownloadTargetCallback with an empty FilePath signals that the
// download should be cancelled.
// If user cancels the file save dialog, run the callback with empty FilePath.
callback.Run(path, download::DownloadItem::TARGET_DISPOSITION_PROMPT,
download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, path,
path.empty() ? download::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED
: download::DOWNLOAD_INTERRUPT_REASON_NONE);
// download should be cancelled. If user cancels the file save dialog, run
// the callback with empty FilePath.
const base::FilePath download_path = result ? path : base::FilePath();
const auto interrupt_reason =
download_path.empty() ? download::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED
: download::DOWNLOAD_INTERRUPT_REASON_NONE;
download_callback.Run(path, download::DownloadItem::TARGET_DISPOSITION_PROMPT,
download::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
download_path, interrupt_reason);
}

void AtomDownloadManagerDelegate::Shutdown() {
@@ -25,10 +25,6 @@ class AtomDownloadManagerDelegate : public content::DownloadManagerDelegate {
explicit AtomDownloadManagerDelegate(content::DownloadManager* manager);
~AtomDownloadManagerDelegate() override;

void OnDownloadPathGenerated(uint32_t download_id,
const content::DownloadTargetCallback& callback,
const base::FilePath& default_path);

// content::DownloadManagerDelegate:
void Shutdown() override;
bool DetermineDownloadTarget(
@@ -45,6 +41,25 @@ class AtomDownloadManagerDelegate : public content::DownloadManagerDelegate {
void GetItemSaveDialogOptions(download::DownloadItem* item,
file_dialog::DialogSettings* settings);

void OnDownloadPathGenerated(uint32_t download_id,
const content::DownloadTargetCallback& callback,
const base::FilePath& default_path);

#if defined(MAS_BUILD)
void OnDownloadSaveDialogDone(
uint32_t download_id,
const content::DownloadTargetCallback& download_callback,
bool result,
const base::FilePath& path,
const std::string& bookmark);
#else
void OnDownloadSaveDialogDone(
uint32_t download_id,
const content::DownloadTargetCallback& download_callback,
bool result,
const base::FilePath& path);
#endif

content::DownloadManager* download_manager_;
base::WeakPtrFactory<AtomDownloadManagerDelegate> weak_ptr_factory_;

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.