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

Ensure HEIC files selectable from “Upload” button #66292

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

adamsilverstein
Copy link
Member

@adamsilverstein adamsilverstein commented Oct 21, 2024

Fixes #66291.

This may be an upstream Chrome bug, see https://core.trac.wordpress.org/ticket/62268

Copy link

github-actions bot commented Oct 21, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: adamsilverstein <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: ironprogrammer <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: ndiego <[email protected]>
Co-authored-by: afercia <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ironprogrammer
Copy link
Contributor

Thank you for the PR, @adamsilverstein!

In my testing, this resolves the issue, and allows HEIC files to be selected for upload on the post screen. ✅

@swissspidy
Copy link
Member

For testing, what if the server doesn‘t support HEIC?

@andrewserong andrewserong added [Type] Bug An existing feature does not function as intended [Feature] Media Anything that impacts the experience of managing media [Block] Image Affects the Image Block Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Oct 22, 2024
@ndiego
Copy link
Member

ndiego commented Oct 22, 2024

For testing, what if the server doesn‘t support HEIC?

Yeah, while this fix addresses the situation where an HEIC image is not selectable, there is still an issue with HEIC images being uploadable when they shouldn't be.

In the video below, I tested in Playground, which does not include ImageMagick. In this situation, I don't think you should be able to upload an HEIC image. Now, it's important to note that this behavior currently exists in 6.6.

So, it's not a regression, but it's still an unfortunate bug that will become more noticeable once 6.7 is out and people start trying to use HEIC images when they are not supported.

heic-bug.mp4

@afercia
Copy link
Contributor

afercia commented Oct 22, 2024

Related: #66293

@Mamaduka
Copy link
Member

Instead of updating all individual uses, maybe we should enhance the FormFileUpload component when the accept prop contains image/*. I believe all accept props from different components are passed down to it at the end.

@adamsilverstein
Copy link
Member Author

Instead of updating all individual uses, maybe we should enhance the FormFileUpload component when the accept prop contains image/*. I believe all accept props from different components are passed down to it at the end.

That makes sense and it would be easier to revert if/when Chrome lands the fix in stable as well. I looked at that originally, but since FormFileUpload could be used elsewhere there would need to be added logic to only add the type when uploading images as you said, adding some complexity.

I can rework with that approach.

@adamsilverstein
Copy link
Member Author

adamsilverstein commented Oct 22, 2024

In the video below, I tested in Playground, which does not include ImageMagick. In this situation, I don't think you should be able to upload an HEIC image. Now, it's important to note that this behavior currently exists in 6.6.

You shouldn't be able to upload HEIC files when the server doesn't support them. This is a completely separate issue from being able to select HEIC images in the file upload button though.

It looks like core media handles this correctly, but not in Gutenberg. This issue should address that: #66293

@@ -18,7 +18,7 @@ import type { FormFileUploadProps } from './types';
*
* const MyFormFileUpload = () => (
* <FormFileUpload
* accept="image/*"
* accept="image/*, image/heic"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we should change this example code here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta [Block] Image Affects the Image Block [Feature] Media Anything that impacts the experience of managing media [Type] Bug An existing feature does not function as intended
Projects
Status: 🔎 Needs Review
Development

Successfully merging this pull request may close these issues.

Chrome: Unable to upload HEIC image through post editor
7 participants