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

Add spec for 'sandboxed downloads' flag #4293

Merged
merged 7 commits into from
Mar 4, 2020
Merged

Conversation

xyaoinum
Copy link
Contributor

@xyaoinum xyaoinum commented Jan 15, 2019

The spec was discussed in #3236.

(See WHATWG Working Mode: Changes for more details.)


/browsing-the-web.html ( diff )
/iframe-embed-object.html ( diff )
/links.html ( diff )
/origin.html ( diff )

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for writing this up. I mostly have editorial comments and a question. The plan here is to disable downloads in sandboxed documents that were not initiated with user activation unless an opt-in flag is set?

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation labels Jan 15, 2019
@xyaoinum
Copy link
Contributor Author

The plan here is to disable downloads in sandboxed documents that were not initiated with user activation unless an opt-in flag is set?

annevk: Yes. Also addressed your other inline comments.

@xyaoinum
Copy link
Contributor Author

…| and relevant specifications.

Add |sandboxed download without user activation browsing context flag| and relevant specifications.

See whatwg#3236 for more context.
…tions.

Add |sandboxed download browsing context flag| and relevant specifications.

See whatwg#3236 for more context.
@xyaoinum xyaoinum changed the title Add spec for download without user activation flag in sandbox Add spec for 'sandboxed downloads' flag Oct 21, 2019
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Picking up to help review this!

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting there!

source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
source Outdated Show resolved Hide resolved
Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this looks good to me. I edited the OP to use the modern pull request template, which @xyaoinum can work on filling out with web platform tests, crbug links, etc. Maybe also edit to add a brief summary of the change, that we can use as a commit message eventually.

We still don't seem to have a second implementer interested, as the discussions in #3236 have stalled, although Mozilla has been engaged throughout (and @johnwilander also commented a year ago!). Thoughts welcome... perhaps it'd be worth Chrome trying to ship this new restriction, and if it's web compatible, others will be more interested?

@annevk
Copy link
Member

annevk commented Mar 4, 2020

Mozilla is interested in these restrictions, not necessarily convinced we need opt-outs, but it's also not a priority for us at the moment. We're happy with Chrome experimenting and also putting it in the specification, with the caveat that removing the flag again might be a thing in the future.

@domenic
Copy link
Member

domenic commented Mar 4, 2020

Thanks very much @annevk! That position makes a lot of sense to me, and I think we're very amenable to removing the allow-downloads flag if it turns out to not be necessary in the wild.

I'll update the OP to fill out the template, and then merge. @xyaoinum, can you file Gecko and WebKit bugs and edit the OP to link to them?

@domenic
Copy link
Member

domenic commented Mar 4, 2020

I'm sorry, the bugs are already filed. Nevermind on that front, @xyaoinum!

@domenic domenic merged commit e910f6a into whatwg:master Mar 4, 2020
@domenic domenic removed the needs implementer interest Moving the issue forward requires implementers to express interest label Mar 4, 2020
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this pull request Mar 13, 2020
LGTMs to ship:
https://groups.google.com/a/chromium.org/g/blink-dev/c/JdAQ6HNoZvk/m/B1aMS33oAAAJ

whatwg/html spec: whatwg/html#4293

Bug: 539938
Change-Id: Ib875fe971ff8ab93ed733f6a0c1d4dd3ee5c7489
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095602
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750209}
@kyuz0
Copy link

kyuz0 commented Jun 9, 2020

With this feature in production, we are now looking for a recommended way web application can implement a very common use-case, that is a download button implemented as an anchor tag.

As we are not directly using any iframes, I cannot see where we would be setting the 'allow-donwloads' flag.

qtprojectorg pushed a commit to qt/qtwebengine-chromium that referenced this pull request Jun 10, 2020
Enabled runtime feature BlockingDownloadsInSandbox

LGTMs to ship:
https://groups.google.com/a/chromium.org/g/blink-dev/c/JdAQ6HNoZvk/m/B1aMS33oAAAJ

whatwg/html spec: whatwg/html#4293

Bug: 539938
Change-Id: Ib875fe971ff8ab93ed733f6a0c1d4dd3ee5c7489
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>
@candrews
Copy link

With this feature in production, we are now looking for a recommended way web application can implement a very common use-case, that is a download button implemented as an anchor tag.

As we are not directly using any iframes, I cannot see where we would be setting the 'allow-donwloads' flag.

I suspect you're using Chrome and experiencing this issue: https://bugs.chromium.org/p/chromium/issues/detail?id=1106906

@nettiopsu
Copy link

I am having an issue with PDF downloads in Chrome even if "allow-downloads" was set. PDF links (e.g. link like https://www.w3.org/WAI/ER/tests/xhtml/testfiles/resources/pdf/dummy.pdf) are not working - it opens a new window (with target=_blank), but the window is blank. There is no any error messages in console. It is supposed to open built-in PDF reader and it might be the reason why it is not working (because it is not exactly a file download, but more like opening a file in this PDF reader). Workaround is to use "allow-popups-to-escape-sandbox" instead

I consider this is a bug, since there is no error messages in console and it breaks Chrome's default behavior for PDF reader...

It might be connected with this issue:
#3958

@domenic
Copy link
Member

domenic commented Aug 6, 2020

@nettiopsu can you file bugs on Chromium on https://crbug.com/new? That sounds like a Chromium issue, not an issue with the HTML Standard.

jvalleroy pushed a commit to freedombox/FreedomBox that referenced this pull request Jan 2, 2021
Closes: #2002.

Entire FreedomBox UI is served within a sandbox that is originally meant for
cross-site iframes.

A newly introduced flag allow-downloads is required to trigger downloads. Two
instances where this is used in FreedomBox is openvpn profiled download and
backup download. Firefox 81 and Chrome 83 implement this flag.

Add 'allow-downloads' to sandbox directives to fix this.

References:

- whatwg/html#4293
- https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe
- https://bugzilla.mozilla.org/show_bug.cgi?id=1558394
- https://www.chromestatus.com/feature/5706745674465280

Tests:

- Check that OpenVPN profile can be downloaded.
- Check that backups can be downloaded.
- Check on Firefox 78 and Chromium 83.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Veiko Aasa <veiko17@disroot.org>
Ninojumilla501

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation topic: sandbox
Development

Successfully merging this pull request may close these issues.

None yet

8 participants