File Block: Remove the download attribute

[Copons]

Description

Remove the download attribute from the File block's "Download" button.

The addition of the download attribute for a tags to the wp_kses whitelist has been recently blocked (cc @pento):

The download attribute doesn't work on cross-origin links (eg, any site that uses a CDN for hosting uploads). I don't know that we necessarily need to account for this, but it is something to consider.

It's also a risk to allow the download filename to be set: for example, an author could upload my_definitely_not_suspicious_file.txt, but then set the download attribute to be CLICK_ME.bat, which isn't great. If we do allow the download attribute, it should only be allowed with no value.

It makes sense to me to just remove the attribute altogether from the File block, and leave to the user the decision of saving the file instead of navigating to it.

How has this been tested?

Manually on a local environment, checking for regressions.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• My code has proper inline documentation.

[talldan]

Thanks for the PR, @Copons!

I noticed when testing this, the 'button' still says download, which is misleading for the user:

I think we should consider removing this button entirely or alternatively changing the default text (could be changed to something like 'open').

The other part that's misleading is the toggle option in the sidebar:

This could be removed or perhaps changed to just 'Show button'.

[talldan]

Removing the download attribute makes the button style link somewhat redundant in the File block. It ends up doing the same thing as the other link.

I wonder if we'd consider removing it - that would probably need a designer's input

[pento]

We don't need to remove the download attribute, we just need to set it with no value.

[talldan]

If we did remove the download button, this selector could be changed back to a

[Copons]

Thanks for the review @talldan, you made some very interesting points.

Removing the download attribute makes the button style link somewhat redundant in the File block. It ends up doing the same thing as the other link.

I'd argue that the button would still have a value as a CTA.

Off the top of my head, I can think of a couple of way to keep the button still useful, even without a behaviour of its own:

• When the block is set to show the button, the filename is not a link.
  E.g. filename.txt <a href="filename.txt">Download</a>

• The button is part of the link, so that when hovered they change style together.
  E.g. <a href="filename.txt">filename.txt <span>Download</span></a>

---

changing the default text [...] to something like 'open'

the toggle option in the sidebar [...] could be [...] changed to just 'Show button'.

Both these points make much sense to me, but of course let's wait for a design look. 🙂

[pento]

We don't need to remove the download attribute entirely. If we just restrict it to being set (but not given a value), which removes the security issues.

For sites that use a CDN for hosting uploads, you could use a file passthrough handler to add the Content-Disposition: attachment header, forcing the file to be a download. In WordPress Multisite, that already exists in the form of ms-files.php. For prehistoric multisites, such as WordPress.com, there's also wp-content/blogs.php, which is a file you should definitely look at. ;)

[pento]

We don't need to remove the download attribute, we just need to set it with no value.

[Copons]

Replaced by #10948