Make WordPress Core

Opened 8 years ago

Last modified 21 months ago

#39625 new defect (bug)

Give an error message when a non-image is uploaded for featured image

Reported by: karmatosed's profile karmatosed Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Media Keywords: has-patch needs-testing needs-screenshots
Focuses: Cc:

Description

I was able to upload a video but it didn't error, it just didn't show. I feel we should give some feedback that this isn't supported. Yes, it doesn't add the video but it shouldn't just reshow the 'add' call. This is what you see after having seemingly gone through upload working - the video shows in media browser.

https://cldup.com/272Cq89Sl0.png

Attachments (5)

text-file-uploaded-for-featured-image.png (132.4 KB) - added by westonruter 6 years ago.
text-file-selected-for-featured-image.png (22.1 KB) - added by westonruter 6 years ago.
39625.diff (2.8 KB) - added by worldweb 6 years ago.
Issue resolved. Now, it if file is not image then it will give error in featured image popup.
featured_image_document_upload_validation.png (81.7 KB) - added by worldweb 6 years ago.
Featured image invalid file validation
featured_image_upload.png (98.6 KB) - added by worldweb 6 years ago.
If uploaded file is valid then it will work as it is.

Download all attachments as: .zip

Change History (31)

#1 @karmatosed
8 years ago

  • Component changed from General to Media
  • Keywords ux-feedback added

#2 @Presskopp
8 years ago

Forgive me, ICNR:
I was able to insert a salami in my drive, but it didn't error, it just didn't play. I feel we should give some feedback that this isn't supported. Yes, it doesn't play the salami but it shouldn't just reshow the 'Insert CD' call. This is what you see after having seemingly gone through mounting working - the salami is mounted as empty disc.

It reads a bit like that because it demands an image, and no video or pdf or mp3. We now could

1) not let the user upload non-image files here at all
2) let them upload whatever is allowed, but give a warning "Filetype not supported, please upload a valid image file - your file has been uploaded anyway and you will find it in the media library, but you still don't have a featured image"
3) open up this function to allow videos as 'featured media' if a theme would allow such
4) leave it as it is

#3 @lukecavanagh
8 years ago

@Presskopp

Option two seems like the most valid one to go with.

#4 @westonruter
8 years ago

I think option 1 is correct. There seems to be a deficiency in the uploader whereby the MIME type passed into the media library is not also being passed along to the uploader. The uploader seems like it should be restricting the file types.

See related issue in media widgets: https://github.com/xwp/wp-core-media-widgets/issues/128

#5 @westonruter
8 years ago

I can see the full list of MIME types when I look at wp.media.frame.uploader.uploader.uploader.settings.filters.mime_types. I'm not sure how to properly override it, however.

I'm not the only one to ask the question: http://stackoverflow.com/questions/38400352/plupload-change-mime-type-filter-after-init

Looks like there is a bug in Plupload affecting the ability to change the file filter after init: https://github.com/moxiecode/plupload/issues/1339

#6 in reply to: ↑ description @SergeyBiryukov
7 years ago

  • Summary changed from Give an error message when a video is uploaded for featured image to Give an error message when a non-image is uploaded for featured image

Replying to karmatosed:

I was able to upload a video but it didn't error, it just didn't show.

Confirmed, the same happens when uploading a PDF, ZIP, or any other non-image file.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


6 years ago

#9 @melchoyce
6 years ago

Did we manage to fix this in the media widgets, @westonruter / @joemcgill?

#10 @westonruter
6 years ago

  • Milestone changed from Awaiting Review to 5.3

This issue is not fixed, no.

Steps to reproduce:

  1. Create a text file on your computer.
  2. In block editor, on Document panel, click Set Featured Image.
  3. Switch to Upload Files tab.
  4. Select the Text file and upload.
  5. 🚫 Fail: The Select button is not disabled, allowing the Text file to be selected as the Featured Image.
  6. 🚫 Fail: The Text file is being used as the src for the featured image in the block editor panel:
<div class="editor-post-featured-image">
	<button type="button" class="components-button editor-post-featured-image__preview" aria-label="Edit or update the image">
		<div class="components-responsive-wrapper">
			<div></div>
			<img src="http://example.com/wp-content/uploads/2013/10/test.txt" alt="" class="components-responsive-wrapper__content">
		</div>
	</button>
	<button type="button" class="components-button is-button is-default is-large">Replace image</button>
	<button type="button" class="components-button is-link is-destructive">Remove featured image</button>
</div>

The uploader needs to respect the file type constraint provided for the media library. For instance, the MediaUpload component needs to use the allowedTypes to constrain the selected uploaded files, for example: https://github.com/WordPress/gutenberg/blob/6d3312a/packages/editor/src/components/post-featured-image/index.js#L53

@worldweb
6 years ago

Issue resolved. Now, it if file is not image then it will give error in featured image popup.

#11 @Presskopp
6 years ago

  • Keywords has-patch added

This ticket was mentioned in Slack in #core by worldweb. View the logs.


6 years ago

This ticket was mentioned in Slack in #core by worldweb. View the logs.


6 years ago

This ticket was mentioned in Slack in #core by worldweb. View the logs.


6 years ago

This ticket was mentioned in Slack in #forums by worldweb. View the logs.


6 years ago

This ticket was mentioned in Slack in #core-themes by worldweb. View the logs.


6 years ago

This ticket was mentioned in Slack in #core-editor by worldweb. View the logs.


6 years ago

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


6 years ago

#19 @garrett-eclipse
6 years ago

  • Keywords needs-testing needs-screenshots added; ux-feedback removed

Thanks for the patch @worldweb discussing in #design Slack (A WordPress.org slack account is required to view that link) today this looks to need some testing and if you could supply screenshots will help the design team conduct a thorough review. Appreciated.

@worldweb
6 years ago

Featured image invalid file validation

@worldweb
6 years ago

If uploaded file is valid then it will work as it is.

This ticket was mentioned in Slack in #core-media by mike. View the logs.


6 years ago

#21 @karmatosed
6 years ago

  • Keywords needs-design-feedback added

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


6 years ago

#23 @kjellr
6 years ago

Hey @worldweb — thanks for providing the screenshots. The design team had another look at this during our triage today:

https://wordpress.slack.com/archives/design/p1565628896461300
(A WordPress.org slack account is required to view that link)

In general, this update looks great. Our only comment is that this could benefit from providing a list of the accepted file types. Is that something we can do here?

Thanks!

#24 @karmatosed
5 years ago

  • Keywords needs-design-feedback removed

#25 @davidbaumwald
5 years ago

  • Milestone changed from 5.3 to Future Release

Based on the latest comments, this ticket still needs some work. With Beta 1 of version 5.3 only a few days away, this is being moved to Future Release. If this ticket can be resolved in time for 5.3, feel free to re-milestone.

This ticket was mentioned in Slack in #core-media by joedolson. View the logs.


21 months ago

Note: See TracTickets for help on using tickets.