hide download button for public preview of audio files

[fstorz]

When the option to hide downloads was selected at public link creation, the download button can be hidden by the audio html attribute controlsList="nodownload"

closes#26086
closes#27872

code was not tested, due to lack of local php setup

[szaimen]

cc @nextcloud/server please review

[tcitworld]

The controlsList attribute is not standard… at the moment at least. Chrome has got it for a while, but both Webkit (Safari) and Firefox voted against.

[fstorz]

• Safari does not show the "Download" button at all, that this PR should fix (Tested on MacOS).
• Don't Safari & Firefox just ignore the attribute when they don't support it?
• According to Mozilla MDN Web Docs also Opera & Edge support the controlsList attribute

[tcitworld]

Yes, it was more about adding code that doesn't follow standards, even if it doesn't break anything that bothered me. It means support for it can be changed/removed at any time.

Opera & Edge use the same rendering engine as Chrome, so their support will always be the same than Chrome (except for feature flags activated at different times).

[szaimen]

Is there maybe another flag that could be added that follows standards?

[fstorz]

The "download" button is only implemented by Chrome (or Chromium based browsers), correct?
So from the discussions in the threads, @tcitworld linked in his comment above, there will be no "standard" in the near future.

Additional discussion on this topic in the "HTML Standard" repository

[szaimen]

Just tested the patch successfully and it works on Chromium based browsers. 👍
The download option doesnt get shown in firefox either way so it doesnt need to work in firefox.

[Pytal]

Looks good to me :) @skjnldsv@blizz@artonge@juliushaertl@ChristophWurst@eneiluj@ArtificialOwl@icewind1991 thoughts?

[blizz]

Please address correctly @blizzz Sent from Yahoo Mail on Android On Fri, Jun 25, 2021 at 5:44 PM, ***@***.***> wrote: @Pytal approved this pull request. Looks good to me :) @skjnldsv@blizz@artonge@juliushaertl@ChristophWurst@eneiluj@ArtificialOwl@icewind1991 thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[artonge]

Not sure about how useful this is.

1. I agree with @tcitworld about this not being a standard and that it will probably be replaced, or removed.
2. Only chromium browsers seems to offer a download button behind a three-dot menu.
3. The user still has the option to download the file by right-clicking on the audio element. So what's next, overriding right-click to prevent that too ?

But, if this comes with a comment pointing to this PR or other relevant info that adds context, then I won't oppose :).

[juliusknorr]

Only chromium browsers seems to offer a download button behind a three-dot menu.
The user still has the option to download the file by right-clicking on the audio element. So what's next, overriding right-click to prevent that too ?

I'd be ok getting this in as it would fit the "hide download" feature which is kind of controversial anyways, as it always was ment to just affect UI elements since there is no real way we can be sure that downloading is not possible anymore (right click, network console, screenshotting, ...) The PR properly hides the element where it is shown, which is chromium only anyways.

[szaimen]

So basically IIRC, @fstorz please add a comment that points to the discussion here in the code and everybody should be fine :)

[LukasReschke]

The user still has the option to download the file by right-clicking on the audio element. So what's next, overriding right-click to prevent that too ?

FWIW: This is what Gmail "Confidential Emails" does. One can still bypass this with the developer console, but it is about making it harder :)

[szaimen]

@fstorz any update here? :)

[fstorz]

@szaimen is there any best practice in the nextcloud repo to add such a comment?

Sample 1

<audio tabindex="0" controls="" preload="none" style="width: 100%; max-width: <?php p($_['previewMaxX']); ?>px; max-height: <?php p($_['previewMaxY']); ?>px"
<?php // See https://github.com/nextcloud/server/pull/27674 ?>
<?php if ($_['hideDownload']) { ?>controlsList="nodownload" <?php } ?>>

Sample 2

<!-- See https://github.com/nextcloud/server/pull/27674 -->
<audio tabindex="0" controls="" preload="none" style="width: 100%; max-width: <?php p($_['previewMaxX']); ?>px; max-height: <?php p($_['previewMaxY']); ?>px"
<?php if ($_['hideDownload']) { ?>controlsList="nodownload" <?php } ?>>

[szaimen]

Maybe @LukasReschke or @juliushaertl can comment on this one ⬆️? Thanks! :)

[LukasReschke]

Both methods would be ok to me. I'd probably tend more to 1.

[szaimen]

/backport to stable22

[szaimen]

/backport to stable21

[szaimen]

/backport to stable20

[welcome]

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

[szaimen]

/backport to stable21

[szaimen]

/backport to stable20