Skip to content

fix: Fixes HEIC uploads hanging indefinitely when ImageDecoder stalls - #81211

Closed
singhakanshu00 wants to merge 11 commits into
WordPress:trunkfrom
singhakanshu00:fix/heic-upload-hang-issue
Closed

singhakanshu00 wants to merge 11 commits into
WordPress:trunkfrom
singhakanshu00:fix/heic-upload-hang-issue

Conversation

@singhakanshu00

Copy link
Copy Markdown
Contributor

What?

Closes #81043

Fixes HEIC image uploads hanging indefinitely in the block editor. The upload spinner would spin forever with no console error, the original .heic file would be saved to the uploads folder, but no resized sub-sizes (150/300/768/1024) would ever be generated.

Why?

As described in #81043, WordPress 7.1's client-side media processing is supposed to fall back to server-side processing transparently when the browser/OS can't decode HEIC client-side. Instead, the upload just hung silently.

Root cause: canvasConvertToJpeg() in packages/upload-media/src/canvas-utils.ts tries three decoding strategies in order — createImageBitmap, WebCodecs ImageDecoder, and HEIC-container-parsing + WebCodecs VideoDecoder. Strategies 1 and 3 are wrapped in try/catch so a failure falls through to the next strategy, but strategy 2 was not.

On platforms where the underlying HEVC codec isn't actually available (e.g. Windows without the HEVC Video Extensions), ImageDecoder.isTypeSupported() can optimistically report support based on container-format recognition alone. When that happens, decoder.decode() doesn't reject — it simply never settles. Since nothing throws, the surrounding try/finally never resolves either, and the await canvasConvertToJpeg(...) call in prepareItem() (private-actions.ts) hangs indefinitely, so dispatch.cancelItem() is never reached. This matches the reported symptom exactly.

How?

In canvas-utils.ts, strategy 2 now:

  • Races decoder.decode() against a 3-second timeout.
  • On timeout, force-closes the decoder — per the WebCodecs spec, closing a decoder rejects any of its pending decode() calls, turning the stuck promise into a real rejection.
  • Is wrapped in try/catch, matching strategies 1 and 3, so a timeout or rejection now falls through to strategy 3 (or the final "browser can't decode HEIC" error) instead of hanging the whole upload.

Testing Instructions

I don't have a Windows machine without the HEVC extension, so I verified this on macOS (Chrome) by forcing the exact failure condition via DevTools, rather than relying on real hardware lacking the codec:

  1. Open the block editor in Chrome or Edge (this bug only applies to Chromium's WebCodecs ImageDecoder; Safari doesn't expose it, so it's unaffected either way).
  2. Open DevTools → Console, and paste this before uploading, to simulate a platform that falsely claims HEIC support:
    const OriginalImageDecoder = window.ImageDecoder;
    class StuckImageDecoder extends OriginalImageDecoder {
    	static async isTypeSupported( type ) {
    		if ( type === 'image/heic' || type === 'image/heif' ) {
    			return true; // pretend the platform supports it
    		}
    		return OriginalImageDecoder.isTypeSupported( type );
    	}
    	decode() {
    		return new Promise( () => {} ); // never resolves or rejects
    	}
    }
    window.ImageDecoder = StuckImageDecoder;
  3. Upload a .heic file via the Image block or Media Library.
  4. On trunk: the spinner hangs indefinitely — reproduces the reported bug.
  5. With this PR: after ~3 seconds it falls through to strategy 3 (real HEVC decode via macOS VideoToolbox) and completes normally — the file converts to .jpg and uploads successfully, with all sub-sizes (150/300/768/1024) generated correctly in the Media Library.

Important caveat: because my Mac has real, working HEVC decode support, my test confirms the timeout/fallback mechanism works, but strategy 3 succeeding is specific to macOS. On a real Windows machine without the HEVC extension (the actual environment in the bug report), VideoDecoder.isConfigSupported() in strategy 3 should correctly report unsupported and fall through to a clean, visible error message instead — rather than a silent hang. I have not been able to confirm this on real Windows hardware, so it would be great if someone could test this PR on a real Windows machine without the HEVC extension installed, to confirm the expected end-to-end behavior (clean error after ~3s, not a spinner).

Testing Instructions for Keyboard

Not applicable — this PR only changes internal upload/decode logic that runs automatically during file upload. It doesn't introduce or modify any UI elements or keyboard interactions.

Screenshots or screencast

Before After
faulty.mov
fixed.mov

Use of AI Tools

Assisted by Claude

@github-actions

github-actions Bot commented Aug 5, 2026 •

Copy link
Copy Markdown

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: singhakanshu00 <akanshusingh@git.wordpress.org>
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: 369work <369work@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: pbiron <pbiron@git.wordpress.org>

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

Comment thread packages/upload-media/src/canvas-utils.ts Outdated

@adamsilverstein adamsilverstein left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Functionally looks good to me, left one small nit feedback.

Not sure how feasible it is, but would be nice to dd a test for this.

@adamsilverstein adamsilverstein added [Type] Bug An existing feature does not function as intended [Feature] Client Side Media Media processing in the browser with WASM labels Aug 5, 2026
@adamsilverstein adamsilverstein self-assigned this Aug 5, 2026
@adamsilverstein adamsilverstein added the Backport to WP 7.1 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Aug 5, 2026
@github-project-automation github-project-automation Bot moved this to 🔎 Needs Review in WordPress 7.1 Editor Tasks Aug 5, 2026
@adamsilverstein adamsilverstein moved this from 🔎 Needs Review to 🏗️ In Progress in WordPress 7.1 Editor Tasks Aug 5, 2026
@369work

369work commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

I tested this PR on my original environment (Windows 11 / Edge, WordPress 7.1-RC1, no HEVC extension).

After applying the fix and re-testing, I found that in my environment the client-side decode path in canvas-utils.ts (including this fix) is never reached at all. detectClientSideMediaSupport() reports SharedArrayBuffer as unavailable due to missing COOP/COEP headers, so processing falls back to server-side before it ever gets to the ImageDecoder logic this PR changes.

So this fix doesn't address the hang in my environment — the hang happens one step earlier, in the fallback-to-server-side path (server-side GD also doesn't support HEIC, so the fallback fails silently too). I've posted more detail on the root cause in #81043.

Wanted to share this in case it's useful context, since the timeout mechanism itself does look correct for the case it's designed to handle (ImageDecoder claiming support it doesn't have).

@369work

369work commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Correction to my comment above — the SharedArrayBuffer/COOP-COEP theory was wrong. I dug further and confirmed directly in my environment: crossOriginIsolated and SharedArrayBuffer are both fine, and detectClientSideMediaSupport() returns {supported: true}. So that's not what's skipping this code.

The actual reason canvasConvertToJpeg() is never reached in my case: prepareItem() decides whether a file is HEIC purely from file.type:

const isImage = file.type.startsWith('image/');
const isHeic = HEIC_MIME_TYPES.includes(file.type);

On my machine (Windows 11, no HEVC Video Extension), File.type for a selected .heic file is "" (empty string) — confirmed via wp.data.subscribe(), item.file.type === '' throughout the whole upload. With file.type === '', both isImage and isHeic evaluate false, so prepareItem() never enters the HEIC branch at all — this PR's fix (and canvasConvertToJpeg() itself) is simply never called. The raw .heic file gets uploaded as-is, and server-side WP_Image_Editor_GD can't process it either, so nothing ever converts it.

The timeout/fallback mechanism in this PR still looks correct for what it's designed to fix (ImageDecoder falsely claiming HEIC support) — it just doesn't reach my specific case, since the file.type check upstream in private-actions.ts filters it out first. Full details posted on #81043.

@adamsilverstein

adamsilverstein commented Aug 6, 2026 •

Copy link
Copy Markdown
Member

On my machine (Windows 11, no HEVC Video Extension), File.type for a selected .heic file is "" (

So I believe in this case, we should show an error. if the windows machine does not have the "Windows HEVC Video Extension" extension, it cannot client side convert the image to JPEG. instead, it should inform the user it is unable to handle the format, and help them forward.

Because HEIC errors are sometimes related to the platform and sometimes the browser and sometimes an extension (as in your case) we have worked to make the errors more specific to help users. See the recent work in #81130.

@adamsilverstein

Copy link
Copy Markdown
Member

Because HEIC errors are sometimes related to the platform and sometimes the browser and sometimes an extension (as in your case) we have worked to make the errors more specific to help users. See the recent work in #81130.

one more thing though, we should break out any string changes to a separate PR if we want to try to land this fix in 7.1 since we are currently in RC and cant add or change strings.

cc: @t-hamano I think we could fix this in a point release (7.1.1) because it is somewhat of an edge case.

@369work

369work commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Makes sense — a clear error is definitely better UX than a silent hang, especially since (as you say) this can come from the platform, browser, or a missing extension, and the user has no way to tell which. Thanks for digging into this and confirming the repro.

Happy to test once there's a build to try. Let me know if there's anything else useful I can check on my end in the meantime.

@singhakanshu00

singhakanshu00 commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor Author

Pushed an additional fix based on @369work's further debugging on the linked issue: on platforms where file.type comes back empty for HEIC files (Windows without the HEVC extension), the file was never being recognized as HEIC at all, skipping this PR's fix entirely and silently falling back to an unhandled upload.

Added a fallback to sniff the .heic/.heif extension in that case, so it correctly routes into the HEIC handling below.

Tested on macOS by simulating an empty file.type directly via the upload store (details in the issue thread) — before the fix, the block never receives an update; after, it hits the HEIC branch and either converts or fails cleanly.

@t-hamano

t-hamano commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

cc: @t-hamano I think we could fix this in a point release (7.1.1) because it is somewhat of an edge case.

This issue always occurs when uploading HEIC images in a default Windows environment where HEVC Video Extension is not enabled, correct?

Based on my experience, there are indeed cases where users want to upload HEIC images on Windows machines. For example, this could involve transferring images taken on an iPhone to a PC via USB, or after syncing images to a PC using iCloud or Google Photos, uploading HEIC images from the PC to a WordPress site. If so, it might be worth fixing by 7.1 RC2.

I will investigate this on a Windows machine today or tomorrow.

@369work

369work commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@singhakanshu00

Tested the updated branch (via the gutenberg-plugin build artifact) on my original environment (Windows 11 / Edge, no HEVC extension). Confirmed: uploading the same HEIC file no longer hangs — the block correctly shows a clear error ("Edge couldn't decode HEIC on this device, so we couldn't convert this one. You can upload a JPEG instead.") and returns to the upload placeholder instead of spinning forever. 🎉

One thing I noticed while testing, probably a separate bug from this fix: an "Upload complete" snackbar appears at the same time as the error (screenshot attached) — but the upload didn't actually complete, it failed. That's not just confusing, it's incorrect — telling the user it succeeded when it didn't. Flagging it since it seems worth its own look, even though it's not blocking this fix.

uptest-log

Uploading upload-test.mp4…

@swissspidy

Copy link
Copy Markdown
Member

One thing I noticed while testing, probably a separate bug from this fix: an "Upload complete" snackbar appears at the same time as the error (screenshot attached) — but the upload didn't actually complete, it failed. That's not just confusing, it's incorrect — telling the user it succeeded when it didn't. Flagging it since it seems worth its own look, even though it's not blocking this fix.

This is already being discussed in #81132

@t-hamano t-hamano left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was able to reproduce the issue, so I will report the details. I am not yet familiar with the architecture of client-side media processing, so please let me know if there is anything incorrect.


Server

I have set up a local environment using Studio. As shown below, HEIC is not supported on the server side.

Details
### wp-core ###

version: 7.1-RC1
site_language: en_US
user_language: en_US
timezone: +00:00
permalink: /%year%/%monthnum%/%day%/%postname%/
https_status: false
multisite: false
user_registration: 0
blog_public: 1
default_comment_status: open
environment_type: local
user_count: 1
dotorg_communication: true

### wp-paths-sizes ###

wordpress_path: D:\_studio\gutenberg
wordpress_size: loading...
uploads_path: D:\_studio\gutenberg/wp-content/uploads
uploads_size: loading...
themes_path: D:\_studio\gutenberg/wp-content/themes
themes_size: loading...
plugins_path: D:\_studio\gutenberg/wp-content/plugins
plugins_size: loading...
fonts_path: D:\_studio\gutenberg/wp-content/uploads/fonts
fonts_size: loading...
database_size: loading...
total_size: loading...

### wp-dropins (1) ###

db.php: true

### wp-active-theme ###

name: Twenty Twenty-Five (twentytwentyfive)
version: 1.5
author: the WordPress team
author_website: https://wordpress.org
parent_theme: none
theme_features: core-block-patterns, post-thumbnails, responsive-embeds, editor-styles, html5, automatic-feed-links, widgets-block-editor, block-templates, post-formats, editor-style
theme_path: D:\_studio\gutenberg/wp-content/themes/twentytwentyfive
auto_update: Disabled

### wp-themes-inactive (2) ###

Twenty Twenty-Four: version: 1.5, author: the WordPress team, Auto-updates disabled
Twenty Twenty-Three: version: 1.6, author: the WordPress team, Auto-updates disabled

### wp-mu-plugins (1) ###

99-studio-loader.php: author: (undefined), version: (undefined)

### wp-plugins-active (1) ###

Gutenberg: version: 23.7.0, author: Gutenberg Team, Auto-updates disabled

### wp-media ###

image_editor: WP_Image_Editor_GD
imagick_module_version: Not available
imagemagick_version: Not available
imagick_version: Not available
file_uploads: 1
post_max_size: 8M
upload_max_filesize: 2M
max_effective_size: 2 MB
max_file_uploads: 20
image_format_transforms: image/heic → image/jpeg, image/heif → image/jpeg, image/heic-sequence → image/jpeg, image/heif-sequence → image/jpeg
gd_version: bundled (2.1.0 compatible)
gd_formats: GIF, JPEG, PNG, WebP, BMP, AVIF, XPM
ghostscript_version: not available

### wp-server ###

server_architecture: Windows NT 10.0 AMD64
httpd_software: PHP/8.4.22 (Development Server)
php_version: 8.4.22 64bit
php_sapi: cli-server
max_input_variables: 1000
time_limit: 30
memory_limit: 128M
admin_memory_limit: 256M
max_input_time: 60
upload_max_filesize: 2M
php_post_max_size: 8M
curl_version: 8.20.0 OpenSSL/3.0.20
suhosin: false
imagick_availability: false
opcode_cache: true
opcode_cache_memory_usage: 41524512 of 132032512
opcode_cache_interned_strings_usage: 72.5% of 8388608 (2306968 free)
opcode_cache_hit_rate: 99.76
opcode_cache_full: false
pretty_permalinks: true
static_robotstxt_file: false
current: 2026-08-06T12:37:56+00:00
utc-time: Thursday, 06-Aug-26 12:37:56 UTC
server-time: 2026-08-06T12:37:53+00:00

### wp-database ###

extension: WP_SQLite_Driver
server_version: 8.0.38
client_version: 3.51.3
max_allowed_packet: undefined
max_connections: undefined

### wp-constants ###

WP_HOME: http://localhost:8881
WP_SITEURL: http://localhost:8881
WP_CONTENT_DIR: D:\_studio\gutenberg/wp-content
WP_PLUGIN_DIR: D:\_studio\gutenberg/wp-content/plugins
WP_MEMORY_LIMIT: 40M
WP_MAX_MEMORY_LIMIT: 256M
WP_DEBUG: true
WP_DEBUG_DISPLAY: false
WP_DEBUG_LOG: true
SCRIPT_DEBUG: false
WP_CACHE: false
CONCATENATE_SCRIPTS: undefined
COMPRESS_SCRIPTS: undefined
COMPRESS_CSS: undefined
WP_ENVIRONMENT_TYPE: local
WP_DEVELOPMENT_MODE: undefined
DB_CHARSET: utf8mb4
DB_COLLATE: undefined
EMPTY_TRASH_DAYS: 30

### wp-filesystem ###

wordpress: writable
wp-content: writable
uploads: writable
plugins: writable
themes: writable
fonts: does not exist
mu-plugins: writable

Browser

The Chrome browser returns the following information regarding the decoder:

await ImageDecoder.isTypeSupported('image/heic');
> false
const decoder = await  VideoDecoder.isConfigSupported({
  codec: 'hvc1.1.6.L93.B0',
  codedWidth: 4032, codedHeight: 3024,
}); 
decoder.supported
> true

OS

I have not installed HEVC Video Extensions.


In this environment, when I upload an HEIC image to the editor, file.type indeed becomes an empty string. Therefore, the approach of inferring the image format from sources other than file.type seems correct.

Comment thread packages/upload-media/src/utils.ts Outdated
Comment on lines +100 to +101
const extension = getFileExtension( name )?.toLowerCase();
return !! extension && extensions.includes( extension );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe inferring file extensions from filenames is unreliable. It might be more accurate to determine if it's an HEIC image from the file buffer, similar to how isAnimatedGif might work.

The following code was generated by AI and has not been tested. @adamsilverstein might have more insights into this area.

/**
 * ISOBMFF brands that identify a HEIC/HEIF image.
 *
 * `mif1`/`msf1` are the generic HEIF brands; they are also listed by AVIF
 * files, which is why the AVIF brands are excluded first.
 */
const HEIC_BRANDS = [ 'heic', 'heix', 'heim', 'heis', 'hevc', 'hevx', 'hevm', 'hevs', 'mif1', 'msf1' ];
const AVIF_BRANDS = [ 'avif', 'avis' ];

/**
 * Detects a HEIC/HEIF file from its ISOBMFF `ftyp` box.
 *
 * Used when the browser can't report a MIME type at all (`file.type === ''`),
 * which happens on platforms lacking the OS-level HEVC codec.
 *
 * @param buffer First bytes of the file (64 is enough for the `ftyp` box).
 * @return Whether the buffer starts with a HEIC/HEIF `ftyp` box.
 */
export function isHeicBuffer( buffer: ArrayBuffer ): boolean {
	const view = new Uint8Array( buffer );

	// A file-type box is at least: size (4) + 'ftyp' (4) + major brand (4).
	if ( view.length < 12 ) {
		return false;
	}

	const readBrand = ( offset: number ) =>
		String.fromCharCode( ...view.subarray( offset, offset + 4 ) );

	if ( readBrand( 4 ) !== 'ftyp' ) {
		return false;
	}

	// Major brand at 8, minor version at 12, compatible brands from 16 on.
	const boxSize = new DataView( buffer ).getUint32( 0 );
	const end = Math.min( boxSize || view.length, view.length );
	const brands = [ readBrand( 8 ) ];
	for ( let offset = 16; offset + 4 <= end; offset += 4 ) {
		brands.push( readBrand( offset ) );
	}

	// AVIF also carries the generic HEIF brands, so rule it out first.
	if ( brands.some( ( brand ) => AVIF_BRANDS.includes( brand ) ) ) {
		return false;
	}

	return brands.some( ( brand ) => HEIC_BRANDS.includes( brand ) );
}
const isHeicByContent =
	! file.type && isHeicBuffer( await file.slice( 0, 64 ).arrayBuffer() );

const isImage = file.type.startsWith( 'image/' ) || isHeicByContent;
const isHeic = HEIC_MIME_TYPES.includes( file.type ) || isHeicByContent;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed in 9991048

@t-hamano t-hamano moved this from 🏗️ In Progress to 🔎 Needs Review in WordPress 7.1 Editor Tasks Aug 6, 2026
@t-hamano

Copy link
Copy Markdown
Contributor

Since it's unlikely that many users will encounter this issue, I decided not to backport this PR to 7.1.0 for now. After the 7.1 release, if it turns out that this issue affects a larger number of users than anticipated, we can consider backporting it to a minor release.

@t-hamano t-hamano removed the Backport to WP 7.1 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Aug 10, 2026
@t-hamano t-hamano moved this from 🔎 Needs Review to 🦵 Punted to 7.2 in WordPress 7.1 Editor Tasks Aug 10, 2026
@t-hamano t-hamano moved this from 🦵 Punted to 7.2 to 🐛 Punted to 7.1.1 in WordPress 7.1 Editor Tasks Aug 10, 2026
@adamsilverstein

Copy link
Copy Markdown
Member

Claude did the digging on a neighboring bug, and it lands here:

Heads up @singhakanshu00 - #81707 turns out to be the same family of bug as this one, and #81737 overlaps with the header check added here.

That issue is a HEIC photo whose extension was changed to .jpg. The browser types it from the extension, so file.type is image/jpeg - non-empty, which is exactly what the ! file.type && gate in prepareItem() excludes. The generalized version in #81737 covers both: it sniffs whenever the declared type leaves the question open, so the empty-file.type case this PR targets falls out of it too. The helper there is closely modeled on isHeicBuffer() from this PR, brand list and AVIF exclusion included.

The ImageDecoder timeout is a separate fix and is not duplicated in #81737 - a decoder that never settles still needs that race, whatever named the file. So whichever PR lands second should drop just its copy of the header check and keep the rest.

One thing found while testing that may be worth knowing for #81043: there is a second place that routes uploads by file.type, heicMediaUpload() in packages/block-editor/src/components/provider/index.js. That is the HEIC-only canvas mode Safari falls into, and it sorts each batch into HEIC and non-HEIC before anything reaches prepareItem(). A file the OS could not type would be sorted into the non-HEIC pile there and handed to the server-side upload, so the fix in this PR may not reach Safari on its own. Untested against the empty-file.type case specifically, so worth confirming rather than taking as read.

Thanks for the work on this one - the header check was the right idea, it just needed a wider gate.

@pbiron

pbiron commented Aug 23, 2026

Copy link
Copy Markdown

Test Report

Patch tested: PATCH_URL

Environment

  • WordPress: 7.1
  • Subdirectory: No
  • PHP: 8.4.21
  • Server: Apache
  • Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.4.21)
  • Browser: Chrome 151.0.0.0
  • OS: Windows 10/11
  • Theme: Twenty Twenty-Five 1.5
  • MU Plugins:
    • None
  • Plugins:
    • Disable Imagick 0.1.0
    • Gutenberg 23.7.1 (with patch applied via the GitHub "build gb plugin" action)
    • Query Monitor 4.0.7
    • Test Reports 1.3.1
  • Image Editors: WP_Image_Editor_GD (active)

Steps taken

  1. Added a core/image block
  2. Clicked on the Upload button and selected an .heic file
  3. This succeeded
  4. Added another core/image block
  5. Clicked on the Media Library button and dragged-dropped the same .heic file into the Media Library
  6. This failed. There was no hanging, but the image wasn't converted to jpg
  7. ✅ Patch is solving the problem in one case / ❌ Patch is failing in another case

Expected result

  • The image would be converted to jpg regardless of whether the Upload or Media Library button were clicked

Screenshots/Screencast with results

81043-test-with-patch

@adamsilverstein

adamsilverstein commented Aug 23, 2026 •

Copy link
Copy Markdown
Member

Clicked on the Media Library button and dragged-dropped the same .heic file into the Media Library

I believe this is using the media library uploader instead of Gutenberg's and this hasn't been upgraded to use client side media yet. This will be fixed in:

I forgot you can do this right inside the editor, I can see how that is confusing. The main flow I was testing is dragging an image directly onto the editor canvas to upload it, this will use the client side path and the image also ends up in your media library.

✅ Patch is solving the problem in one case

Thanks for confirming.

@t-hamano

t-hamano commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

@adamsilverstein The situation is a bit complicated, and I'm confused. Should we test and merge this PR or #81043 first? Also, should both PRs be backported to the 7.1 minor release?

@adamsilverstein

Copy link
Copy Markdown
Member

@adamsilverstein The situation is a bit complicated, and I'm confused. Should we test and merge this PR or #81043 first? Also, should both PRs be backported to the 7.1 minor release?

This PR fixes #81043 - did you mean to link to a different PR @t-hamano?

@singhakanshu00 can you work on resolving the merge conflicts here and I will give the code a review.

@adamsilverstein adamsilverstein left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall looks good to me, Claude raised a few points worth looking into in its review I will post below.

@adamsilverstein

Copy link
Copy Markdown
Member

Thanks for sticking with this one @singhakanshu00. Here is the code review I promised - the header sniffing is the right approach and matches what @369work confirmed on Windows, so this is mostly small stuff on top of the conflict resolution.

Claude went through the code for this one, findings below:

The new test file will fail CI. packages/upload-media/src/test/is-heic-buffer.ts uses describe/it/expect without importing them. test/unit/vitest.config.mjs sets globals: false, and the file isn't in the Jest allowlist in test/unit/test-migration.json, so it runs under Vitest where those aren't globals and the suite throws before the first assertion. All 18 of the other test files in that directory import them, eg. the closest sibling is-animated-gif.ts opens with:

import { describe, expect, it } from 'vitest';

mif1 and msf1 in HEIC_BRANDS match more than HEIC. The docblock notes these are the generic HEIF brands and rules out AVIF, but AVIF isn't the only other format carrying them. Running isHeicBuffer() against synthetic headers:

avci + mif1 + miaf   AVC-coded still image in HEIF    true
msf1 + hevc          HEIF image sequence              true
avif + mif1          AVIF                             false

Neither of the first two is HEIC. With an empty file.type they now route into the HEIC path, fail parseHeic(), and surface the "couldn't decode HEIC" error instead of being handed to the server. Dropping the two generic brands and keeping the hei*/hev* entries avoids that, at the cost of a HEIC that declares only mif1 as its major brand.

The compatible-brand test passes without exercising compatible brands. buildFtyp( 'mif1', 'miaf', 'heic' ) puts mif1 in the major brand slot, and mif1 is itself in HEIC_BRANDS, so the assertion is satisfied before the scan loop ever runs. Deleting the loop outright keeps all four tests green. A non-HEIC major brand exercises it:

buildFtyp( 'mp42', 'heic' )   with the loop: true   without: false

The new catch covers more than the decode. It wraps the OffscreenCanvas/drawImage/convertToBlob block as well as the Promise.race. On trunk a failure in that block propagated out and landed on the IMAGE_TRANSCODING_ERROR branch in prepareItem() - "could not be converted, try JPEG". Now it's swallowed and falls through to strategy 3, so on a machine without VideoDecoder support it ends at HeicUnsupportedError and tells the user their browser can't decode HEIC, which is the misattribution the comments in that file are written to avoid. Moving the catch to wrap only the race keeps the two apart. Strategy 1 has the same shape already, so this is pre-existing rather than introduced here.

Is 3 seconds enough headroom? The race applies to every decode, not only a stalled one, so a large HEIC on slow or loaded hardware can be cut off mid-decode and the result thrown away. That isn't silent - the file is re-read and retried through strategy 3 - but on a HEIC variant parseHeic() can't handle, the user gets "The HEIC file could not be parsed" for a file the browser was decoding fine. Since the bug being guarded is a promise that never settles rather than a slow one, a much larger ceiling would still catch it.

Minor: the byteOffset/byteLength arguments to new DataView() are always 0 and the full length, since view is built over the whole buffer.

The test imports are the only thing I'd call blocking. The rest are worth a look but I don't want to hold this up over them, they are very minor.

@t-hamano

Copy link
Copy Markdown
Contributor

@adamsilverstein The situation is a bit complicated, and I'm confused. Should we test and merge this PR or #81043 first? Also, should both PRs be backported to the 7.1 minor release?

This PR fixes #81043 - did you mean to link to a different PR @t-hamano?

Sorry, I meant this PR and #81737. Which PR should we review first?

@adamsilverstein

Copy link
Copy Markdown
Member

Sorry, I meant this PR and #81737. Which PR should we review first?

@t-hamano Oh, right - these two PRs address separate GitHub Issues, but I suspect the underlying issue is the same. Maybe we can combine them into a single PR. I will review them to see if they can be combined and still fix both reported issues.

adamsilverstein pushed a commit that referenced this pull request Sep 16, 2026
`ImageDecoder.isTypeSupported()` can report HEIC support from the
container format alone. When the platform codec is missing, `decode()`
then never settles, and the upload hangs with no error. Race the decode
against a timeout that closes the decoder, which rejects the pending
call, and catch the rejection so the conversion falls through to the
next strategy instead of stalling the upload.

Folded in from #81211.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wRcDnK65wbQKCjKoie65o
@adamsilverstein

Copy link
Copy Markdown
Member

Hey @singhakanshu00, thanks for sticking with this one and for the timeout idea - it turned out to be the right guard, and you caught it before anyone had the environment to observe it.

Claude did the comparison of the two branches, here is where it landed:

With #81707 turning out to be the same bug seen from the wrong-extension side, the two PRs converged on the same header check, and #81737 already carried the wider gate plus the Safari routing fix and e2e coverage. Rather than have both open, the timeout from this PR is now part of #81737 as your own commit, 66c4e0b, so the authorship stays with you. A follow-up commit there narrows the catch so only a timed-out decode falls through, since #81123 landed in the meantime and wants a decode that rejects on a damaged file reported as a processing error rather than blamed on the codec. The 3 second ceiling went up to 15 for the reason in the review above.

That leaves this PR superseded by #81737, which now closes both #81043 and #81707. The review notes above still apply there and were addressed in the fold, including the test imports.

I'm proposing we close this one in favor of #81737 - please shout if you'd rather carry it yourself, happy to go that way instead. Either way, review eyes on #81737 would be welcome.

@adamsilverstein

Copy link
Copy Markdown
Member

@t-hamano I have rolled all of the changes into a single PR #81737 which could use a review.

@singhakanshu00 I'm closing this one as it is superseded by #81737. Appreciate your review there if possible and I'll make sure to credit you when the fix lands.

@aduth

aduth commented Sep 18, 2026

Copy link
Copy Markdown
Member

Hey 👋 I wanted to give you a heads-up since this pull request is affected by recent validation changes for changelog files.

#83043 adds additional validation for changelog files. You'll note that this pull request is currently failing a "Required changes from trunk" check.

What you'll need to do: You will need to either rebase or merge the latest code from trunk. In addition, a cursory review of open pull requests identified this pull request as potentially failing under the new validation checks. You will want to double-check that any changes to CHANGELOG.md files follow the Maintaining Changelogs guidance, which has been improved as part of these recent changes.

@adamsilverstein

Copy link
Copy Markdown
Member

Closing this one in favor of #81737

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] Client Side Media Media processing in the browser with WASM [Type] Bug An existing feature does not function as intended

Projects

Development

Successfully merging this pull request may close these issues.

HEIC upload hangs indefinitely instead of falling back to server-side processing

8 participants