Skip to content

Commit dcb6aa4

Browse files
adamsilversteinswissspidyandrewserongockham
authored andcommitted
Fix client-side media file naming (#75817)
* Fix client-side media file naming The original filename was stripped and replaced with a generic prefix (e.g., "image.jpeg", "document.pdf") during client-side media processing. Two issues caused this: 1. convertBlobToFile() used instanceof File which fails for cross-realm File objects from the iframe where the block editor canvas renders. Now checks for the name property as a fallback to preserve the original filename. 2. generateThumbnails() referenced attachment.media_filename which doesn't exist. The REST API field is "filename". Thumbnails now correctly use the server-provided filename. * re-enable client sifde media as naming issue is resolved * expect client side media to be enabled * complete removal of disabling shim * Fix client-side media scaled image handling Upload the original file unscaled, then create and sideload the -scaled version after sub-sizes. This matches WordPress core's server-side behavior: original_image metadata is set correctly, sub-sizes derive names from the original, and wp_unique_filename no longer adds -1 to -scaled filenames. * Always register sideload route with scaled enum The parent class already registers the sideload route without 'scaled' in its enum, causing a 400 error. Always register our version so WordPress can fall through to the handler that accepts the 'scaled' image_size value. * Override core sideload route to accept scaled size Core's handler validates first and rejects 'scaled' before Gutenberg's appended handler is tried. Passing override=true replaces core's route entirely with ours. * Add backport changelog for core PR #11015 * Fix PHPCS equals sign alignment in sideload_item * Fix PHPCS equals sign alignment in sideload_image Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add tests for scaled image sideload metadata Verify that the sideload endpoint correctly handles the -scaled image flow: original_image meta is set, attached file is updated, dimensions/filesize are recorded, and wp_get_original_image_path resolves to the original. * improve doc block * Use ResizeCrop queue step for scaled image Move the scaled image resize from an inline vipsResizeImage call to a proper ResizeCrop queue operation. This ensures the resize respects maxConcurrentImageProcessing limits and follows the same pattern used for thumbnail generation. * Add thorough e2e tests for client-side media Replace weak conditional tests with four unconditional tests covering filename preservation, scaled image sideloading, thumbnail base filename correctness, and below-threshold no-op behavior. Add uploadWithName() helper to enable filename-aware assertions. * Remove re-enabling of client-side media from this PR Move the re-enabling of client-side media processing to a separate PR so this PR focuses only on the file naming fix. Restores the disabling shim, bootstrap test override, and reverts the e2e tests to the trunk version. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Restore improved e2e tests for client-side media file naming These tests validate the file naming fix, not the re-enabling, so they belong in this PR. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org> Co-authored-by: swissspidy <swissspidy@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
1 parent a1350dd commit dcb6aa4

File tree

8 files changed

+640
-158
lines changed

8 files changed

+640
-158
lines changed

backport-changelog/7.0/11015.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
https://github.com/WordPress/wordpress-develop/pull/11015
2+
3+
* https://github.com/WordPress/gutenberg/pull/75817

lib/media/class-gutenberg-rest-attachments-controller.php

Lines changed: 53 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -20,41 +20,44 @@ class Gutenberg_REST_Attachments_Controller extends WP_REST_Attachments_Controll
2020
public function register_routes(): void {
2121
parent::register_routes();
2222

23-
// Only register the sideload route if the parent class doesn't already have it.
24-
if ( ! method_exists( get_parent_class( $this ), 'sideload_item' ) ) {
25-
$valid_image_sizes = array_keys( wp_get_registered_image_subsizes() );
26-
27-
// Special case to set 'original_image' in attachment metadata.
28-
$valid_image_sizes[] = 'original';
29-
// Used for PDF thumbnails.
30-
$valid_image_sizes[] = 'full';
31-
32-
register_rest_route(
33-
$this->namespace,
34-
'/' . $this->rest_base . '/(?P<id>[\d]+)/sideload',
23+
// Override the parent's sideload route so that 'scaled' is included
24+
// in the image_size enum. Without the override, core's handler
25+
// validates first and rejects 'scaled' before ours is tried.
26+
$valid_image_sizes = array_keys( wp_get_registered_image_subsizes() );
27+
28+
// Special case to set 'original_image' in attachment metadata.
29+
$valid_image_sizes[] = 'original';
30+
// Client-side big image threshold: sideload the scaled version.
31+
$valid_image_sizes[] = 'scaled';
32+
// Used for PDF thumbnails.
33+
$valid_image_sizes[] = 'full';
34+
35+
register_rest_route(
36+
$this->namespace,
37+
'/' . $this->rest_base . '/(?P<id>[\d]+)/sideload',
38+
array(
3539
array(
36-
array(
37-
'methods' => WP_REST_Server::CREATABLE,
38-
'callback' => array( $this, 'sideload_item' ),
39-
'permission_callback' => array( $this, 'sideload_item_permissions_check' ),
40-
'args' => array(
41-
'id' => array(
42-
'description' => __( 'Unique identifier for the attachment.', 'gutenberg' ),
43-
'type' => 'integer',
44-
),
45-
'image_size' => array(
46-
'description' => __( 'Image size.', 'gutenberg' ),
47-
'type' => 'string',
48-
'enum' => $valid_image_sizes,
49-
'required' => true,
50-
),
40+
'methods' => WP_REST_Server::CREATABLE,
41+
'callback' => array( $this, 'sideload_item' ),
42+
'permission_callback' => array( $this, 'sideload_item_permissions_check' ),
43+
'args' => array(
44+
'id' => array(
45+
'description' => __( 'Unique identifier for the attachment.', 'gutenberg' ),
46+
'type' => 'integer',
47+
),
48+
'image_size' => array(
49+
'description' => __( 'Image size.', 'gutenberg' ),
50+
'type' => 'string',
51+
'enum' => $valid_image_sizes,
52+
'required' => true,
5153
),
5254
),
53-
'allow_batch' => $this->allow_batch,
54-
'schema' => array( $this, 'get_public_item_schema' ),
55-
)
56-
);
57-
}
55+
),
56+
'allow_batch' => $this->allow_batch,
57+
'schema' => array( $this, 'get_public_item_schema' ),
58+
),
59+
true // Override core's route so 'scaled' is included in the enum.
60+
);
5861
}
5962

6063
/**
@@ -210,6 +213,8 @@ public function create_item( $request ) {
210213
// Disable server-side EXIF rotation so the client can handle it.
211214
// This preserves the original orientation value in the metadata.
212215
add_filter( 'wp_image_maybe_exif_rotate', '__return_false', 100 );
216+
// Disable server-side big image scaling since the client handles it.
217+
add_filter( 'big_image_size_threshold', '__return_zero', 100 );
213218
}
214219

215220
if ( ! $request['convert_format'] ) {
@@ -221,6 +226,7 @@ public function create_item( $request ) {
221226
remove_filter( 'intermediate_image_sizes_advanced', '__return_empty_array', 100 );
222227
remove_filter( 'fallback_intermediate_image_sizes', '__return_empty_array', 100 );
223228
remove_filter( 'wp_image_maybe_exif_rotate', '__return_false', 100 );
229+
remove_filter( 'big_image_size_threshold', '__return_zero', 100 );
224230
remove_filter( 'image_editor_output_format', '__return_empty_array', 100 );
225231

226232
return $response;
@@ -275,7 +281,7 @@ private static function filter_wp_unique_filename( $filename, $dir, $number, $at
275281
}
276282

277283
$matches = array();
278-
if ( preg_match( '/(.*)(-\d+x\d+)-' . $number . '$/', $name, $matches ) ) {
284+
if ( preg_match( '/(.*)(-\d+x\d+|-scaled)-' . $number . '$/', $name, $matches ) ) {
279285
$filename_without_suffix = $matches[1] . $matches[2] . ".$ext";
280286
if ( $matches[1] === $orig_name && ! file_exists( "$dir/$filename_without_suffix" ) ) {
281287
return $filename_without_suffix;
@@ -381,6 +387,20 @@ public function sideload_item( WP_REST_Request $request ) {
381387

382388
if ( 'original' === $image_size ) {
383389
$metadata['original_image'] = wp_basename( $path );
390+
} elseif ( 'scaled' === $image_size ) {
391+
// The current attached file is the original; record it as original_image.
392+
$current_file = get_attached_file( $attachment_id, true );
393+
$metadata['original_image'] = wp_basename( $current_file );
394+
395+
// Update the attached file to point to the scaled version.
396+
update_attached_file( $attachment_id, $path );
397+
398+
$size = wp_getimagesize( $path );
399+
400+
$metadata['width'] = $size ? $size[0] : 0;
401+
$metadata['height'] = $size ? $size[1] : 0;
402+
$metadata['filesize'] = wp_filesize( $path );
403+
$metadata['file'] = _wp_relative_upload_path( $path );
384404
} else {
385405
$metadata['sizes'] = $metadata['sizes'] ?? array();
386406

packages/upload-media/src/store/private-actions.ts

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -717,22 +717,7 @@ export function prepareItem( id: QueueItemId ) {
717717

718718
// For images that can be processed by vips, check if we need to scale down based on threshold.
719719
if ( isImage && isVipsSupported ) {
720-
const { bigImageSizeThreshold, imageOutputFormats } = settings;
721-
722-
// If a threshold is set, add a resize operation to scale down large images.
723-
// This matches WordPress core's behavior in wp_create_image_subsizes().
724-
if ( bigImageSizeThreshold ) {
725-
operations.push( [
726-
OperationType.ResizeCrop,
727-
{
728-
resize: {
729-
width: bigImageSizeThreshold,
730-
height: bigImageSizeThreshold,
731-
},
732-
isThresholdResize: true,
733-
},
734-
] );
735-
}
720+
const { imageOutputFormats } = settings;
736721

737722
// Check if we need to transcode to a different format.
738723
// Uses WordPress image_editor_output_format filter settings.
@@ -1126,8 +1111,8 @@ export function generateThumbnails( id: QueueItemId ) {
11261111
// Use sourceFile for thumbnail generation to preserve quality.
11271112
// WordPress core generates thumbnails from the original (unscaled) image.
11281113
// Vips will auto-rotate based on EXIF orientation during thumbnail generation.
1129-
const file = attachment.media_filename
1130-
? renameFile( item.sourceFile, attachment.media_filename )
1114+
const file = attachment.filename
1115+
? renameFile( item.sourceFile, attachment.filename )
11311116
: item.sourceFile;
11321117
const batchId = uuidv4();
11331118

@@ -1205,6 +1190,54 @@ export function generateThumbnails( id: QueueItemId ) {
12051190
operations: thumbnailOperations,
12061191
} );
12071192
}
1193+
1194+
// Create and sideload the scaled version.
1195+
const { bigImageSizeThreshold } = settings;
1196+
if ( bigImageSizeThreshold && attachment.id ) {
1197+
// Rename sourceFile to match the server attachment filename.
1198+
const sourceForScaled = attachment.filename
1199+
? renameFile( item.sourceFile, attachment.filename )
1200+
: item.sourceFile;
1201+
1202+
// Add scaling to queue.
1203+
const scaledOperations: Operation[] = [
1204+
[
1205+
OperationType.ResizeCrop,
1206+
{
1207+
resize: {
1208+
width: bigImageSizeThreshold,
1209+
height: bigImageSizeThreshold,
1210+
},
1211+
isThresholdResize: true,
1212+
},
1213+
],
1214+
];
1215+
1216+
// Add transcoding if format conversion is configured.
1217+
if ( thumbnailTranscodeOperation ) {
1218+
scaledOperations.push( thumbnailTranscodeOperation );
1219+
}
1220+
1221+
scaledOperations.push( OperationType.Upload );
1222+
1223+
dispatch.addSideloadItem( {
1224+
file: sourceForScaled,
1225+
onChange: ( [ updatedAttachment ] ) => {
1226+
if ( isBlobURL( updatedAttachment.url ) ) {
1227+
return;
1228+
}
1229+
item.onChange?.( [ updatedAttachment ] );
1230+
},
1231+
batchId,
1232+
parentId: item.id,
1233+
additionalData: {
1234+
post: attachment.id,
1235+
image_size: 'scaled',
1236+
convert_format: false,
1237+
},
1238+
operations: scaledOperations,
1239+
} );
1240+
}
12081241
}
12091242

12101243
dispatch.finishOperation( id, {} );

packages/upload-media/src/store/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,6 @@ export interface Attachment {
193193
mime_type: string;
194194
featured_media?: number;
195195
missing_image_sizes?: string[];
196-
media_filename?: string;
197196
poster?: string;
198197
/**
199198
* EXIF orientation value from the original image.
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/**
2+
* Internal dependencies
3+
*/
4+
import { convertBlobToFile } from '../utils';
5+
6+
describe( 'convertBlobToFile', () => {
7+
it( 'should return a File unchanged', () => {
8+
const file = new File( [ 'content' ], 'photo.jpg', {
9+
type: 'image/jpeg',
10+
} );
11+
const result = convertBlobToFile( file );
12+
expect( result ).toBe( file );
13+
expect( result.name ).toBe( 'photo.jpg' );
14+
} );
15+
16+
it( 'should convert a Blob with a default name', () => {
17+
const blob = new Blob( [ 'content' ], { type: 'image/png' } );
18+
const result = convertBlobToFile( blob );
19+
expect( result ).toBeInstanceOf( File );
20+
expect( result.name ).toBe( 'image.png' );
21+
expect( result.type ).toBe( 'image/png' );
22+
} );
23+
24+
it( 'should use "document" prefix for PDF blobs', () => {
25+
const blob = new Blob( [ 'content' ], { type: 'application/pdf' } );
26+
const result = convertBlobToFile( blob );
27+
expect( result.name ).toBe( 'document.pdf' );
28+
} );
29+
30+
it( 'should preserve the name from cross-realm File objects', () => {
31+
// Simulate a cross-realm File object (e.g., from an iframe).
32+
// These objects have a `name` property but fail `instanceof File`.
33+
const crossRealmFile = new Blob( [ 'content' ], {
34+
type: 'image/jpeg',
35+
} );
36+
Object.defineProperty( crossRealmFile, 'name', {
37+
value: 'IMG_7977.jpg',
38+
writable: false,
39+
} );
40+
Object.defineProperty( crossRealmFile, 'lastModified', {
41+
value: 1700000000000,
42+
writable: false,
43+
} );
44+
45+
const result = convertBlobToFile( crossRealmFile );
46+
expect( result ).toBeInstanceOf( File );
47+
expect( result.name ).toBe( 'IMG_7977.jpg' );
48+
expect( result.type ).toBe( 'image/jpeg' );
49+
expect( result.lastModified ).toBe( 1700000000000 );
50+
} );
51+
52+
it( 'should preserve the name for non-image cross-realm File objects', () => {
53+
const crossRealmFile = new Blob( [ 'content' ], {
54+
type: 'application/pdf',
55+
} );
56+
Object.defineProperty( crossRealmFile, 'name', {
57+
value: 'my-document.pdf',
58+
writable: false,
59+
} );
60+
61+
const result = convertBlobToFile( crossRealmFile );
62+
expect( result ).toBeInstanceOf( File );
63+
expect( result.name ).toBe( 'my-document.pdf' );
64+
expect( result.type ).toBe( 'application/pdf' );
65+
} );
66+
} );

packages/upload-media/src/utils.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ import { _x } from '@wordpress/i18n';
99
*
1010
* If it is already a File object, it is returned unchanged.
1111
*
12+
* Handles cross-realm File objects (e.g., from iframes) that have a `name`
13+
* property but fail `instanceof File` checks because the File constructor
14+
* differs between browsing contexts.
15+
*
1216
* @param fileOrBlob Blob object.
1317
* @return File object.
1418
*/
@@ -17,6 +21,20 @@ export function convertBlobToFile( fileOrBlob: Blob | File ): File {
1721
return fileOrBlob;
1822
}
1923

24+
// Handle cross-realm File objects (e.g., from iframes where the block
25+
// editor canvas renders). These objects have a `name` property but fail
26+
// the `instanceof File` check because each browsing context has its own
27+
// File constructor.
28+
if (
29+
'name' in fileOrBlob &&
30+
typeof ( fileOrBlob as File ).name === 'string'
31+
) {
32+
return new File( [ fileOrBlob ], ( fileOrBlob as File ).name, {
33+
type: fileOrBlob.type,
34+
lastModified: ( fileOrBlob as File ).lastModified,
35+
} );
36+
}
37+
2038
// Extension is only an approximation.
2139
// The server will override it if incorrect.
2240
const ext = fileOrBlob.type.split( '/' )[ 1 ];

0 commit comments

Comments
 (0)