Opened 4 years ago
Closed 3 years ago
#53626 closed enhancement (fixed)
Change Media upload failures string to not include "Security" messaging for each failure
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.9 | Priority: | normal |
Severity: | normal | Version: | 5.7 |
Component: | Media | Keywords: | good-first-bug has-patch |
Focuses: | Cc: |
Description
Currently when uploading a media item that is not supported, the default message claims that the reason it cannot upload is due to security reasons. This is not always true. We should expand on this to be either conditional to show security message on valid security risks, or just a general "this file type is not supported" message for all types uploaded. I lean more toward the general message.
Attachments (6)
Change History (22)
#3
@
4 years ago
- Keywords has-patch added; needs-patch removed
I found 3 occurrences and changed them to the string which is already there
#4
@
4 years ago
The "not permitted for security reasons" version of the string also appears in Gutenberg, so an upstream PR to change that would be good.
"Sorry, this file type is not supported here." looks to be about someone trying to upload an incompatible file to a block (eg an image to the audio block).
For file types the user is prevented from uploading, I'm wondering if a more direct "Sorry, you are not allowed to upload this file type." is better. No security FUD if it's prevented because the site owner simply doesn't want to use a particular mime type on their site.
Additionally...
Is there a compelling reason this can't be held off to 5.9? Typically string changes are avoided as much as possible in minor releases to avoid the additional translation load on the polyglots.
If it is a must have, then I suggest going with 53626.diff in the 5.8 branch and creating a follow up ticket if you wish to go with a new string for 5.9.
#5
@
3 years ago
- Milestone changed from 5.8.1 to 5.8.2
Ideally, once the desired string is found, the Gutenberg string is fixed at the same time. With 5.8.1 RC in less than 24 hours, I'm going to punt this to allow for more time.
#6
@
3 years ago
Not sure if a decision was made about the right string for this ticket @peterwilsoncc. I like your proposed version "Sorry, you are not allowed to upload this file type." but I'm not sure if I should make a patch. (?) I'm brand new to contributing.
Just adding that I was able to reproduce the issue reported by @antpb with the following settings:
- Updated my local wordpress-svn copy to revision 51905 (14 Oct 2021)
- Ran MAMP version 6.6 (1211) (on MacOS Catalina 10.15.7)
- Tested the bug and applied the patch (53626.diff) running PHP version 7.4.21 AND version 8.0.8
GIF Animation Images attached to the ticket
@
3 years ago
In case we decide to go with @peterwilsoncc 's suggestion... I went ahead and created a patch. Hope I'm not cluttering this ticket.
@
3 years ago
In case we decide to go with @peterwilsoncc 's suggestion... I went ahead and created a patch. Hope I'm not cluttering this ticket. It might be necessary to coordinate with the Gutenberg team to update this string in their codebase as mentioned by @desrosj below.
#7
@
3 years ago
@iluy
Thank you for making a patch, you're always welcome to do so on any ticket and it will never be considered cluttering, I assure you! Let's see if we can get this committed and get a core contributor badge against your profile :)
In 53626.3.diff I've made a minor change to include a forth occurrence of the string in the wordpress-develop repository. Otherwise it's identical to 53626.2.2.diff.
#8
@
3 years ago
@peterwilsoncc thanks for the feedback and the support words. :)
Hope we'll get this ticket committed soon. I'll keep looking for some more good first bugs in the coming days. Cheers!
#10
@
3 years ago
- Keywords needs-testing removed
- Milestone changed from 5.8.2 to 5.9
This is looking good, but it still needs a pull request accompanying it for Gutenberg.
Since 5.8.1 RC is later today, I'm going to punt this to 5.9 so both can be merged at the same time.
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
3 years ago
#14
@
3 years ago
I opened a PR for the Gutenberg side changes: https://github.com/WordPress/gutenberg/pull/36226
We already have a string "Sorry, this file type is not supported here." in