Conversation
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +59 B (0%) Total Size: 6.84 MB
ℹ️ View Unchanged
|
|
@andrewserong - this PR fixed the naming bug we noticed. Currently working to address the |
Looks like this originated from |
|
Flaky tests detected in cd14fcb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22300109647
|
|
I have resolved the issue with the scaled image naming and creation; these changes will require an additional backport PR which I am working on now. |
|
Core backport: WordPress/wordpress-develop#11015 |
|
I implemented the naming in media-experiments the other way around. Core: a large image is resized to fit threshold, the "large" version you can use gets a -scaled suffix. The unscaled original has no suffix. This PR implies both -scaled and -original versions will be uploaded, which doesn't make sense under these circumstances. |
| const scaledFile = await vipsResizeImage( | ||
| item.id, | ||
| sourceForScaled, | ||
| { | ||
| width: bigImageSizeThreshold, | ||
| height: bigImageSizeThreshold, | ||
| }, | ||
| false, // smartCrop | ||
| false, // addSuffix | ||
| item.abortController?.signal, | ||
| true // scaledSuffix | ||
| ); |
There was a problem hiding this comment.
Image operations like resizing should never happen "inline" like this. It should go through a proper upload queue step. That's what the ResizeCrop above was for.
There was a problem hiding this comment.
updated the approach
|
A bit fuzzy on the details, but IIRC this was just much easier to implement. Scale down image, upload all thumbnails, and then upload the unmodified original with the prefix. No hassle with suffixes for thumbnails or anything. I don't mind parity with the server-side logic but then it needs some thorough e2e tests IMO. |
There was a problem hiding this comment.
This is testing nicely for me so far! Looking at the uploaded files on my file-system, they all preserve the prefix as expected with client side media activated or deactivated:
Other files (pdf, mp4, zip) continue to behave as expected when uploaded, too.
I can also safely disable client-side media processing via adding add_filter( 'wp_client_side_media_processing_enabled', '__return_false' ); to a separate plugin, and that works as expected 👍
Since this PR is proposing re-enabling client-side media process by default, should we also re-enable the cross-origin isolation e2e tests that I skipped in #75764?
And did we want to add checks to skip if the user is on a low-powered or low-network device in this PR, or leave it for a follow-up?
let me open a separate PR explicitly to re-enable client-side media process by default, and also re-enable the cross-origin isolation e2e tests. That will keep this PR a little smaller and we can try to add other fixes as well.
Yes, good idea. I think we may also want to disable for Safari and Firefox initially until we can resolve any glaring compatibility issues (so fr: tinymce and maintenence mode are reported effected). |
The improved e2e tests for file naming belong in #75817 instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
andrewserong
left a comment
There was a problem hiding this comment.
Still testing great for me! Looks like the e2e test failures are unrelated 👍
This looks good to merge to me, nice work fixing this up! 🎉
| const items = uploadStore.getItems(); | ||
| return items.length === 0; | ||
| }, | ||
| { timeout: 120000 } |
There was a problem hiding this comment.
Tiniest of tiny nits, please feel free to ignore. Is 2 minutes a long time for this timeout?
|
I'm planning to publish another Gutenberg RC today, and I'd be happy to include this fix. @adamsilverstein Would you mind rebasing? Ideally we can merge with CI all-green. |
|
Nevermind, I'll do a quick rebase myself :) |
The improved e2e tests for file naming belong in #75817 instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Re-enable client-side media processing in the Gutenberg plugin Remove the disabling shim that was added in #75756, re-enable the cross-origin isolation e2e tests skipped in #75764, and add thorough e2e tests for client-side media filename handling, scaled image metadata, and thumbnail generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove e2e test changes from this PR The improved e2e tests for file naming belong in #75817 instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Clean up dead bootstrap filter refs in tests The bootstrap override for client-side media processing was removed, but tests still referenced it with remove_filter / add_filter calls. Also replaces redundant "can be enabled" test with a "can be disabled via filter" test. --------- Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org> Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
* Re-enable client-side media processing in the Gutenberg plugin Remove the disabling shim that was added in #75756, re-enable the cross-origin isolation e2e tests skipped in #75764, and add thorough e2e tests for client-side media filename handling, scaled image metadata, and thumbnail generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove e2e test changes from this PR The improved e2e tests for file naming belong in #75817 instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Clean up dead bootstrap filter refs in tests The bootstrap override for client-side media processing was removed, but tests still referenced it with remove_filter / add_filter calls. Also replaces redundant "can be enabled" test with a "can be disabled via filter" test. --------- Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org> Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
* 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>
|
I just cherry-picked this PR to the release/22.6 branch to get it included in the next release: 89864fb |
* Re-enable client-side media processing in the Gutenberg plugin Remove the disabling shim that was added in #75756, re-enable the cross-origin isolation e2e tests skipped in #75764, and add thorough e2e tests for client-side media filename handling, scaled image metadata, and thumbnail generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove e2e test changes from this PR The improved e2e tests for file naming belong in #75817 instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Clean up dead bootstrap filter refs in tests The bootstrap override for client-side media processing was removed, but tests still referenced it with remove_filter / add_filter calls. Also replaces redundant "can be enabled" test with a "can be disabled via filter" test. --------- Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org> Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
|
@ellatrix This should probably be included in WP 7.0 as well, no? |
|
Probably. @adamsilverstein could you apply the |
* 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>
|
I just cherry-picked this PR to the wp/7.0 branch to get it included in the next release: dcb6aa4 |
CI run: #11059. See #64595. --- I've included a log of the Gutenberg changes with the following command: git log --reverse --format="- %s" 23b566c72e9c4a36219ef5d6e62890f05551f6cb..022d8dd3d461f91b15c1f0410649d3ebb027207f | sed 's|#\([0-9][0-9]*\)|https://github.com/WordPress/gutenberg/pull/\1|g; /github\.com\/WordPress\/gutenberg\/pull/!d' | pbcopy - Pattern Editing: Fix nested patterns/sections (WordPress/gutenberg#75772) - QuickEdit: rename status label and remove extra labels in popup (WordPress/gutenberg#75824) - Fix block editing modes not recomputing when isolated editor value changes (WordPress/gutenberg#75821) - Synced patterns: Fix block editing mode of synced pattern content when nested in an unsynced pattern (WordPress/gutenberg#75818) - Block Support: Fix custom CSS not saved when style schema is not defined (WordPress/gutenberg#75797) - Gallery: Fixes keyboard focus escaping the lightbox overlay when navigating a gallery with Tab/Shift+Tab. (WordPress/gutenberg#75852) - Navigation Overlay Close: Set Close as default text, rather than using a placeholder (WordPress/gutenberg#75692) - RTC: Fix entity save call / initial persistence. (WordPress/gutenberg#75841) - Real-time collaboration: Improve collaboration within the same rich text (WordPress/gutenberg#75703) - Client Side Media: Add device/browser capability detection (WordPress/gutenberg#75863) - Navigation editing: simplify edit/view buttons (WordPress/gutenberg#75819) - Add core/icon block to theme.json schema (WordPress/gutenberg#75813) - Fix error when undoing newly added pattern (WordPress/gutenberg#75850) - Page List Item: Replace RawHTML with dangerouslySetInnerHTML for label and title (WordPress/gutenberg#75890) - REST API: Make filter_wp_unique_filename() static to match core, plus avoid duplicate routes (WordPress/gutenberg#75782) - RichText: useAnchor: Fix TypeError in virtual element (WordPress/gutenberg#75900) - DataViews: Remove menu divider again. (WordPress/gutenberg#75908) - Theme: Add build plugins to inject design token fallbacks (WordPress/gutenberg#75901) - Theme: Remove global stylesheet (WordPress/gutenberg#75879) - Real-time collaboration: Remove ghost awareness state explicitly when refreshing (WordPress/gutenberg#75883) - Real-time collaboration: Expand mergeCrdtBlocks() automated testing (WordPress/gutenberg#75923) - Fix client-side media file naming (WordPress/gutenberg#75817) - Add: Connectors screen (WordPress/gutenberg#75833) - Merge document meta into state map (WordPress/gutenberg#75830) - Move WordPress meta key from sync package to core-data (WordPress/gutenberg#75846) - Bugfix: Fix casing of getPersistedCRDTDoc (WordPress/gutenberg#75922) - Add debug logging to SyncManager (WordPress/gutenberg#75924) - DataForm: fix label colors (WordPress/gutenberg#75730) - DataViews: minimize padding for primary action buttons (WordPress/gutenberg#75721) (WordPress/gutenberg#75947) - Connectors: Add `_ai_` prefix to connector setting names and fix naming inconsistencies (WordPress/gutenberg#75948) - Connectors: Unhook Core callbacks in Gutenberg coexistence (WordPress/gutenberg#75935) - Unsynced patterns: Rename 'Disconnect pattern' to 'Detach pattern' in context menu (WordPress/gutenberg#75807) - Editor: Remove View dropdown and pinned items from revisions header (WordPress/gutenberg#75951) - Fix: Template revisions infinite spinner (WordPress/gutenberg#75953) - Backport: Avoid flickering while refreshing (WordPress/gutenberg#74572) (WordPress/gutenberg#75952) - Add wp_ prefix to real time collaberation option. (WordPress/gutenberg#75837) git-svn-id: https://develop.svn.wordpress.org/trunk@61750 602fd350-edb4-49c9-b593-d223f7449a82
CI run: WordPress/wordpress-develop#11059. See #64595. --- I've included a log of the Gutenberg changes with the following command: git log --reverse --format="- %s" 23b566c72e9c4a36219ef5d6e62890f05551f6cb..022d8dd3d461f91b15c1f0410649d3ebb027207f | sed 's|#\([0-9][0-9]*\)|https://github.com/WordPress/gutenberg/pull/\1|g; /github\.com\/WordPress\/gutenberg\/pull/!d' | pbcopy - Pattern Editing: Fix nested patterns/sections (WordPress/gutenberg#75772) - QuickEdit: rename status label and remove extra labels in popup (WordPress/gutenberg#75824) - Fix block editing modes not recomputing when isolated editor value changes (WordPress/gutenberg#75821) - Synced patterns: Fix block editing mode of synced pattern content when nested in an unsynced pattern (WordPress/gutenberg#75818) - Block Support: Fix custom CSS not saved when style schema is not defined (WordPress/gutenberg#75797) - Gallery: Fixes keyboard focus escaping the lightbox overlay when navigating a gallery with Tab/Shift+Tab. (WordPress/gutenberg#75852) - Navigation Overlay Close: Set Close as default text, rather than using a placeholder (WordPress/gutenberg#75692) - RTC: Fix entity save call / initial persistence. (WordPress/gutenberg#75841) - Real-time collaboration: Improve collaboration within the same rich text (WordPress/gutenberg#75703) - Client Side Media: Add device/browser capability detection (WordPress/gutenberg#75863) - Navigation editing: simplify edit/view buttons (WordPress/gutenberg#75819) - Add core/icon block to theme.json schema (WordPress/gutenberg#75813) - Fix error when undoing newly added pattern (WordPress/gutenberg#75850) - Page List Item: Replace RawHTML with dangerouslySetInnerHTML for label and title (WordPress/gutenberg#75890) - REST API: Make filter_wp_unique_filename() static to match core, plus avoid duplicate routes (WordPress/gutenberg#75782) - RichText: useAnchor: Fix TypeError in virtual element (WordPress/gutenberg#75900) - DataViews: Remove menu divider again. (WordPress/gutenberg#75908) - Theme: Add build plugins to inject design token fallbacks (WordPress/gutenberg#75901) - Theme: Remove global stylesheet (WordPress/gutenberg#75879) - Real-time collaboration: Remove ghost awareness state explicitly when refreshing (WordPress/gutenberg#75883) - Real-time collaboration: Expand mergeCrdtBlocks() automated testing (WordPress/gutenberg#75923) - Fix client-side media file naming (WordPress/gutenberg#75817) - Add: Connectors screen (WordPress/gutenberg#75833) - Merge document meta into state map (WordPress/gutenberg#75830) - Move WordPress meta key from sync package to core-data (WordPress/gutenberg#75846) - Bugfix: Fix casing of getPersistedCRDTDoc (WordPress/gutenberg#75922) - Add debug logging to SyncManager (WordPress/gutenberg#75924) - DataForm: fix label colors (WordPress/gutenberg#75730) - DataViews: minimize padding for primary action buttons (WordPress/gutenberg#75721) (WordPress/gutenberg#75947) - Connectors: Add `_ai_` prefix to connector setting names and fix naming inconsistencies (WordPress/gutenberg#75948) - Connectors: Unhook Core callbacks in Gutenberg coexistence (WordPress/gutenberg#75935) - Unsynced patterns: Rename 'Disconnect pattern' to 'Detach pattern' in context menu (WordPress/gutenberg#75807) - Editor: Remove View dropdown and pinned items from revisions header (WordPress/gutenberg#75951) - Fix: Template revisions infinite spinner (WordPress/gutenberg#75953) - Backport: Avoid flickering while refreshing (WordPress/gutenberg#74572) (WordPress/gutenberg#75952) - Add wp_ prefix to real time collaberation option. (WordPress/gutenberg#75837) Built from https://develop.svn.wordpress.org/trunk@61750 git-svn-id: http://core.svn.wordpress.org/trunk@61056 1a063a9b-81f0-0310-95a4-ce76da25c4cd
* Re-enable client-side media processing in the Gutenberg plugin Remove the disabling shim that was added in #75756, re-enable the cross-origin isolation e2e tests skipped in #75764, and add thorough e2e tests for client-side media filename handling, scaled image metadata, and thumbnail generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove e2e test changes from this PR The improved e2e tests for file naming belong in #75817 instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Clean up dead bootstrap filter refs in tests The bootstrap override for client-side media processing was removed, but tests still referenced it with remove_filter / add_filter calls. Also replaces redundant "can be enabled" test with a "can be disabled via filter" test. --------- Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org> Co-authored-by: ockham <bernhard-reiter@git.wordpress.org>
* Client-side media processing: Disable in Gutenberg just for now (#75756) * Client-side media processing: Disable in Gutenberg just for now * Try to fix failing test * Update docblock Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org> * Re-enable client-side media processing (#75848) * Re-enable client-side media processing in the Gutenberg plugin Remove the disabling shim that was added in #75756, re-enable the cross-origin isolation e2e tests skipped in #75764, and add thorough e2e tests for client-side media filename handling, scaled image metadata, and thumbnail generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Remove e2e test changes from this PR The improved e2e tests for file naming belong in #75817 instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Clean up dead bootstrap filter refs in tests The bootstrap override for client-side media processing was removed, but tests still referenced it with remove_filter / add_filter calls. Also replaces redundant "can be enabled" test with a "can be disabled via filter" test. --------- Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>

Summary
Fixes #75302
When the client-side media experiment is active, uploading or drag-and-dropping files causes the original filename to be stripped and replaced with a generic prefix (
image.jpeg,document.pdf). This PR fixes two bugs:instanceof Filefailure: The block editor canvas renders inside an iframe. File objects from the iframe failinstanceof Filein the parent window because each browsing context has its ownFileconstructor.convertBlobToFile()now checks for anameproperty as a fallback, preserving the original filename for cross-realm File objects.generateThumbnails()referencedattachment.media_filenamewhich doesn't exist — the REST API field isfilename. Thumbnails now correctly use the server-provided filename when renaming source files.Also adds a
convert-blob-to-file.tsunit test.Test plan
IMG_7977.jpg)IMG_7977.jpeg,IMG_7977-1024x768.jpeg)my-document.pdf) and verify it retains its original name