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 #16612
Conversation
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> ```
|
||
auto dialog_callback = | ||
base::Bind(&AtomDownloadManagerDelegate::OnDownloadSaveDialogDone, | ||
base::Unretained(this), download_id, callback); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using base::Unretained(this)
here, but I think it should be fine since AtomDownloadManagerDelegate should exist until shutdown. @zcbenz, is this safe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it should be fine in this case.
Release Notes Persisted
|
I was unable to backport this PR to "4-0-x" cleanly; |
I have automatically backported this PR to "5-0-x", please check out #16640 |
I believe the existing code was fine, but better be safe than sorry. This typo was introduced in #16612.
I believe the existing code was fine, but better be safe than sorry. This typo was introduced in electron#16612.
I believe the existing code was fine, but better be safe than sorry. This typo was introduced in #16612.
I believe the existing code was fine, but better be safe than sorry. This typo was introduced in electron#16612.
I believe the existing code was fine, but better be safe than sorry. This typo was introduced in electron#16612.
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=...>
byswitching to the asynchronous API.
To test this:
main.js:
test.html:
Notes: Fix broken save dialog on macOS for
<a>
downloads