What
Follow-up to #77506 capturing two pieces of feedback from @andrewserong:
1. HEIC filename suffix drift vs. generated JPEGs
Noted in this review.
When the client-side HEIC → JPEG conversion runs, the stored filename of the original HEIC and the generated JPEG(s) can drift out of sync:
- Upload
sample-IMG_0218.heic → stored as sample-IMG_0218-1.heic on the server.
- The generated JPEG is stored as
sample-IMG_0218.jpg (no -1 suffix).
- On a subsequent upload of the same file, the HEIC picks up
-3 while the JPEGs get -2, so the basenames no longer match across the set.
Expected: the HEIC original and the generated JPEG (and its sub-sizes) should share a consistent basename / numeric suffix so they stay associated on the server filesystem.
Screenshot from Andrew showing the drift across two uploads:
<img width="1014" height="372" alt="HEIC vs JPG suffix drift" src="https://github.com/user-attachments/assets/ed41a5f6-c149-4fe8-84b4-319d5418b16b\" />
This was initially thought to be related to #77036, but it still reproduces after that landed — so it appears to be a separate problem in the HEIC upload path introduced in / around #76731.
2. `convert_format` is not declared on the sideload route
Noted in this comment.
The client dispatches `convert_format: false` as `additionalData` when sideloading a scaled derivative:
|
dispatch.addSideloadItem( { |
|
file: sourceForScaled, |
|
batchId, |
|
parentId: item.id, |
|
additionalData: { |
|
post: attachment.id, |
|
image_size: 'scaled', |
|
convert_format: false, |
|
}, |
|
operations: scaledOperations, |
|
} ); |
The `sideload_item` handler reads `$request['convert_format']` at `lib/media/class-gutenberg-rest-attachments-controller.php:548`, but the sideload route's `args` declaration only lists `id`, `image_size`, and `generate_sub_sizes`:
|
'args' => array( |
|
'id' => array( |
|
'description' => __( 'Unique identifier for the attachment.', 'gutenberg' ), |
|
'type' => 'integer', |
|
), |
|
'image_size' => array( |
|
'description' => __( 'Image size. Can be a single size name or an array of size names to register the same file under multiple sizes.', 'gutenberg' ), |
|
'type' => array( 'string', 'array' ), |
|
'items' => array( |
|
'type' => 'string', |
|
), |
|
'required' => true, |
|
// A custom callback is used instead of the default `rest_validate_request_arg` |
|
// because WordPress's `rest_is_array()` treats scalar strings as single-element |
|
// lists (via wp_parse_list), so a oneOf with both a string and array schema |
|
// matches a plain string twice and validation fails with "matches more than one |
|
// of the expected formats". The callback validates the enum per-item using the |
|
// current list of registered sizes, which reflects any sizes added after the |
|
// route was registered (e.g. via add_image_size() in tests). |
|
'validate_callback' => static function ( $value, $request, $param ) { |
|
$valid_sizes = array_keys( wp_get_registered_image_subsizes() ); |
|
$valid_sizes[] = 'original'; |
|
$valid_sizes[] = 'original-heic'; |
|
$valid_sizes[] = 'scaled'; |
|
$valid_sizes[] = 'full'; |
|
|
|
$items = is_string( $value ) ? array( $value ) : ( is_array( $value ) ? $value : null ); |
|
if ( null === $items ) { |
|
return new WP_Error( |
|
'rest_invalid_type', |
|
/* translators: %s: Parameter name. */ |
|
sprintf( __( '%s must be a string or an array of strings.', 'gutenberg' ), $param ) |
|
); |
|
} |
|
|
|
foreach ( $items as $item ) { |
|
if ( ! is_string( $item ) || ! in_array( $item, $valid_sizes, true ) ) { |
|
return new WP_Error( |
|
'rest_not_in_enum', |
|
/* translators: %s: Parameter name. */ |
|
sprintf( __( '%s contains an invalid image size.', 'gutenberg' ), $param ) |
|
); |
|
} |
|
} |
|
|
|
return true; |
|
}, |
|
), |
|
'generate_sub_sizes' => array( |
|
'description' => __( 'Whether to generate image sub sizes from the sideloaded file.', 'gutenberg' ), |
|
'type' => 'boolean', |
|
'default' => false, |
|
), |
|
), |
|
), |
Because `convert_format` is not declared with `'type' => 'boolean'`, the REST controller will not sanitize the incoming multipart/form value from a string ("false") to a PHP boolean. `if ( ! $request['convert_format'] )` then evaluates truthy on the string `"false"`, and the `image_editor_output_format` filter is never added — meaning server-side format conversion is not suppressed for sideloads as intended.
The equivalent arg is already declared for the `create_item` path in `get_endpoint_args_for_item_schema()` (lines 200–204). It should be added to the sideload route's `args` array as well, with `'type' => 'boolean'` and a sensible default.
Why
- File 1 breaks the implicit "HEIC original + generated JPEG share a basename" invariant that users (and any tooling that pairs originals with derivatives) rely on.
- File 2 silently disables an optimization the client-side path depends on: format conversion is meant to happen client-side only, but without proper type coercion the server still performs it for sideloaded sub-sizes.
How
Two independent fixes, can land as In a single PR to close this issue:
- Align the unique-filename suffix logic in the HEIC upload path so the `.heic` original and generated `.jpg`(s) share a consistent basename. Investigate whether `wp_unique_filename()` is being called separately per file (via the create and sideload endpoints) rather than as a coordinated set, and whether the `unique_filename` filter workaround used for sideload sub-sizes (`class-gutenberg-rest-attachments-controller.php` ~L556–L580) needs to extend to the HEIC original.
- Add a `convert_format` boolean arg to the sideload route registration in `class-gutenberg-rest-attachments-controller.php` (lines 31–85), mirroring the `generate_sub_sizes` entry.
Note: if possible, e2e tests should verify the file naming after an actual heic upload.
Testing instructions
- Upload the same HEIC image twice and confirm the `.heic` and generated `.jpg` files share the same basename / numeric suffix on the server filesystem.
- ideally compare generated files and naming to a server processing upload on a system that supports HEIC on the server. Should be the default for heic uploads, using the media library is another route for testing.
- With a debugger or logging in `sideload_item`, confirm that `$request['convert_format']` is received as a PHP `false` (not the string `"false"`) and that the `image_editor_output_format` filter is added as expected.
cc @andrewserong
What
Follow-up to #77506 capturing two pieces of feedback from @andrewserong:
1. HEIC filename suffix drift vs. generated JPEGs
Noted in this review.
When the client-side HEIC → JPEG conversion runs, the stored filename of the original HEIC and the generated JPEG(s) can drift out of sync:
sample-IMG_0218.heic→ stored assample-IMG_0218-1.heicon the server.sample-IMG_0218.jpg(no-1suffix).-3while the JPEGs get-2, so the basenames no longer match across the set.Expected: the HEIC original and the generated JPEG (and its sub-sizes) should share a consistent basename / numeric suffix so they stay associated on the server filesystem.
Screenshot from Andrew showing the drift across two uploads:
<img width="1014" height="372" alt="HEIC vs JPG suffix drift" src="https://github.com/user-attachments/assets/ed41a5f6-c149-4fe8-84b4-319d5418b16b\" />
This was initially thought to be related to #77036, but it still reproduces after that landed — so it appears to be a separate problem in the HEIC upload path introduced in / around #76731.
2. `convert_format` is not declared on the sideload route
Noted in this comment.
The client dispatches `convert_format: false` as `additionalData` when sideloading a scaled derivative:
gutenberg/packages/upload-media/src/store/private-actions.ts
Lines 1297 to 1307 in 70387ef
The `sideload_item` handler reads `$request['convert_format']` at `lib/media/class-gutenberg-rest-attachments-controller.php:548`, but the sideload route's `args` declaration only lists `id`, `image_size`, and `generate_sub_sizes`:
gutenberg/lib/media/class-gutenberg-rest-attachments-controller.php
Lines 31 to 85 in 70387ef
Because `convert_format` is not declared with `'type' => 'boolean'`, the REST controller will not sanitize the incoming multipart/form value from a string ("false") to a PHP boolean. `if ( ! $request['convert_format'] )` then evaluates truthy on the string `"false"`, and the `image_editor_output_format` filter is never added — meaning server-side format conversion is not suppressed for sideloads as intended.
The equivalent arg is already declared for the `create_item` path in `get_endpoint_args_for_item_schema()` (lines 200–204). It should be added to the sideload route's `args` array as well, with `'type' => 'boolean'` and a sensible default.
Why
How
Two independent fixes, can land as In a single PR to close this issue:
Note: if possible, e2e tests should verify the file naming after an actual heic upload.
Testing instructions
cc @andrewserong