Tests: Re-enable external-image upload e2e under CSM (server-side sideload)#79605
Draft
adamsilverstein wants to merge 23 commits into
Draft
Tests: Re-enable external-image upload e2e under CSM (server-side sideload)#79605adamsilverstein wants to merge 23 commits into
adamsilverstein wants to merge 23 commits into
Conversation
…atest-base # Conflicts: # package-lock.json # test/storybook-playwright/package.json
The root package.json still pinned @playwright/test at ^1.58.2 while the workspaces were bumped to ^1.61.0, failing 'lint:deps' (syncpack) which requires a single version across the repo.
Bring the playwright-upgrade base current with trunk (was 63 commits behind). Resolve test/e2e/package.json and package-lock.json conflicts: keep the @playwright/test 1.61.0 bump and trunk's new @flakiness/playwright dependency.
The editor screen sends Document-Isolation-Policy: isolate-and-credentialless for cross-origin isolation. This places the editor tab and an already-open preview tab in separate agent clusters, so reusing a preview tab and synchronously accessing previewWindow.document to write the interstitial throws a SecurityError. The preview then keeps showing stale content. Reset the reused tab to about:blank (which returns it to the opener's agent cluster) and poll until its document is reachable before writing the interstitial, instead of accessing the isolated document directly. The interstitial is treated as a progressive enhancement and skipped if the document never becomes reachable; the preview still navigates to the real content. This surfaced via the Playwright 1.60+ upgrade, which ships Chrome 148.
The Playwright 1.60 upgrade ships Chrome for Testing 148, which has a regression in the cross-origin isolated `isolate-and-credentialless` Document-Isolation-Policy runtime that Gutenberg sends on editor screens. Under it three suites fail although the product behaves correctly on shipping Chrome: client-side media processing silently falls back to the server (so format/rotation/sub-size assertions break), and the preload and Loading Patterns specs never reach a settled state so they time out. Gate these specs on the major Chromium version so they skip on 148+ until the browser regression is resolved, mirroring the existing 137+ gate used for Document-Isolation-Policy. See #78632.
The skip comments claimed CSM "silently falls back to the server" and read as a user-facing product regression. Manual testing shows shipping Google Chrome processes client-side media correctly (AVIF upload verified on stable 149 and Canary 151); the failures are confined to the Chrome for Testing build Playwright bundles in CI after the 148/149 bump. Reword the comments to state the verified, CI-specific nature and avoid asserting an unconfirmed mechanism.
The Chromium >=148 skip comments attributed the failures to a Document-Isolation-Policy regression. Investigation shows that is not the cause: DIP is what first enables cross-origin isolation in the CI browser (Chrome <148 never became crossOriginIsolated, so CSM was inactive and the CSM specs simply skipped). With CSM now active under automation, uploads hit a timing-sensitive race in the multi-threaded wasm-vips worker - the decoder intermittently receives a short buffer and libheif aborts, surfacing as IMAGE_TRANSCODING_ERROR. The same wasm-vips decodes the same fixtures in Node and in manual Chrome, so it is automation-timing, not a user regression. Reword the CSM skip to describe the race and link the tracking issue, and reword the preload/performance skips (a separate, not-yet-root-caused cross-origin-isolation startup timeout) to drop the inaccurate "DIP is broken" wording. Tracking: #79377
Drop the `Chromium >= 148` skip gate on the client-side media processing suite. The gate's rationale (a timing-sensitive wasm-vips decode race) is incorrect: wasm-vips processes correctly on Chrome 149 across main-thread, nested-worker, COOP/COEP and Document-Isolation-Policy isolation. The suite was only ever skipped because Chrome < 148 never became cross-origin isolated, so it never ran in CI. The follow-up commit aligns the test expectations with the feature's actual behavior so the suite passes when it runs.
These CSM e2e tests never ran in CI before: they skip unless the browser is cross-origin isolated, which only happens once Document-Isolation-Policy is active. Now that they run, several assert behavior the feature does not implement rather than any Chrome 148 / wasm-vips regression. (Verified wasm-vips processes correctly on Chrome 149 across main-thread, nested-worker, COOP/COEP and Document-Isolation-Policy isolation.) - UltraHDR probe: resolve wasm-vips from @wordpress/vips so the dynamic import works in a clean CI install where it does not hoist to the repo root node_modules. - PNG->JPEG / JPEG->WebP: CSM, like core, only transcodes generated sub-sizes, never the full-size attachment's MIME type. Assert the sub-size format instead of the main file's. - srcset: the editor's default size is `large`, so the block stores the large sub-size URL (a registered size that satisfies srcset matching), not the -scaled full file. Assert a finalized real-file URL plus the front-end srcset, and capture the attachment ID before navigating to the front end where window.wp.data is unavailable. - EXIF auto-rotation: CSM does not bake EXIF orientation into the full-size file (it only sideloads a rotated original_image). Mark fixme pending a product decision on whether to rotate the main file like core.
The `auto-rotates images based on EXIF orientation` test never actually ran in CI (the CSM suite only began executing once Chrome 148 enabled Document-Isolation-Policy), so two problems went unnoticed: - Its fixture `1024x768_e2e_test_image_rotated.jpeg` carried no EXIF orientation tag at all, so nothing could ever rotate it. - It asserted the full-size attachment is baked-rotated to 768x1024. CSM does not do this: `create_item` disables `wp_image_maybe_exif_rotate`, so the stored full-size keeps its pixels plus the EXIF tag (browsers honor it) and rotation is applied to the generated sub-sizes instead. Replace it with coverage that matches actual behavior: - JPEG: server reads EXIF, reports `exif_orientation` (edit context), and sub-sizes come out rotated. Fixture regenerated with a real orientation tag that `exif_read_data()` can read. - AVIF with a native HEIF `irot` transform: wasm-vips/libheif honors the transform and rotates sub-sizes even though the server can't read the container's orientation. - AVIF with EXIF-only orientation: `test.fixme` documenting the gap where neither the server nor libheif applies the rotation. Tracked in #79383. HEIC rotation is not added: HEIC decode relies on platform HEVC via WebCodecs, which is unavailable on the Linux Chromium used in CI.
Revert the CSM e2e rework (un-skipping the suite on Chromium 148, realigning format/rotation expectations, and expanding EXIF-orientation coverage) that accumulated on this branch during CI debugging. The base AVIF decode test fails under the bundled Chrome for Testing 148/149 because of the cross-origin-isolated wasm-vips decode race tracked in #78632, which is a CI browser regression rather than anything this preview-fix PR changes. Restore the `chromiumVersion >= 148` skip gate so the CSM suite skips on the upgraded CI browser, keeping this PR scoped to the preview interstitial fix. The EXIF sub-size rotation coverage lives in #79384, which targets trunk.
…rite The SecurityError under Document-Isolation-Policy is thrown by accessing the reused preview tab's `document`, not by writing the interstitial. Extract a `getPreviewDocument` helper that retries just that access and returns the reachable document (or null), so `writeInterstitialMessage` and markup generation run once, outside the polling loop.
…luate Playwright already knows which browser and version it drives, so read it from the `browser` fixture (`browserType().name()` and `version()`) rather than round-tripping a `page.evaluate` user-agent parse. The helper becomes synchronous; the CSM utility reaches the browser through `page.context().browser()`.
Re-enable the client-side media processing e2e suite on Chromium 148+ (drop the skip gate added for the Playwright/Chrome upgrade) and correct the test expectations that were exposed when CSM first became active in CI. There is no Chromium 148 or wasm-vips regression: an isolation harness on real Chrome 149 runs every wasm-vips op (rotate, thumbnail, format convert) successfully across main-thread, Web Worker, COOP/COEP, Document-Isolation- Policy `isolate-and-credentialless`, and ArrayBuffer-transfer contexts. The suite simply never ran in CI before (Chrome < 148 has no cross-origin isolation, so CSM stayed inactive and the tests skipped), so several assertions that never matched real CSM behavior were never caught. Corrections: - Resolve `wasm-vips` via `@wordpress/vips` so the UltraHDR probe import works in a clean CI install where the dependency is not hoisted to the root. - PNG-to-JPEG / JPEG-to-WebP: `image_editor_output_format` governs only the generated sub-sizes; the full-size attachment keeps its original MIME type, matching core. Assert the sub-size MIME/URL instead of the main file. - srcset: capture the attachment ID while the editor data store is still loaded (before navigating to the front end) and assert the block settles on a finalized uploaded URL rather than a transient blob URL. EXIF orientation handling is intentionally left as `test.fixme` here; the client-side EXIF sub-size rotation fix and its coverage land separately in PR #79384.
With client-side media processing active on Chromium 148+, the "Upload to Media Library" flow for an external image URL routes through the client-side blob pipeline, which does not yet finalize to a /wp-content/uploads/ URL, so the existing assertion no longer holds. Client-side external-image upload is addressed in #79407; skip the test here and re-introduce CSM-aware coverage there.
…affic The preload specs asserted the exact set of REST routes fired during editor startup, gating the assertion on `page.waitForLoadState( 'networkidle' )`. That wait never settles once client-side media processing is active, which on Chromium 148+ happens because the editor loads cross-origin isolated via Document-Isolation-Policy: CSM eagerly creates the `@wordpress/vips` Web Worker from a `blob:` URL, and Chromium keeps that worker's `script` request in-flight for the worker's lifetime, so the page is never network-idle. The specs were therefore skipped on Chromium 148+. The stalling request is a worker `script`, not a `fetch`, so it never enters the recorded request set; the REST traffic the assertions depend on does settle. Replace `networkidle` with `waitForRequestsToSettle()`, which waits for the recorded fetch requests to go quiet, and drop the Chromium 148+ skip. Verified on Chromium 149 with CSM active: both specs pass and the assertions capture the same request set, stable across repeated runs.
The Loading Patterns site-editor measurement was skipped on Chromium 148+ on the assumption that cross-origin isolation prevented the site editor from settling. That attribution was wrong: the test fails identically with Document-Isolation-Policy disabled, so isolation is not the cause. The real cause is that the pattern previews are rendered lazily. Each preview's `Editor canvas` iframe is only created once its list item is scrolled into view, but the test awaited all four previews at once, so the off-screen ones never rendered and `getByTitle( 'Editor canvas' )` timed out. Scroll each pattern option into view before waiting for its preview, and drop the misattributed Chromium 148+ skip (and the now-unused version helper). Verified on Chromium 149: the test passes in ~30s, stable across repeated runs.
…ad' into try/csm-external-image-e2e
…sideload The 'should upload external image to media library' e2e test was skipped on Chromium 148+ because, under cross-origin isolation, the browser fetch of the remote image's bytes is blocked, so 'Upload to Media Library' never finalized to a /wp-content/uploads/ URL. With the server-side sideload in #79409 (the mediaSideloadFromUrl editor setting), the server downloads and sideloads the remote URL, so the upload no longer depends on the browser reading cross-origin bytes. Re-enable the test; it is now CSM-agnostic and passes whether or not the editor is isolated.
github-actions
Bot
added
[Package] Editor
This branch carries the server-side sideload PHP changes from #79409, so the backport-changelog check requires this PR's URL on the entry as well.
adamsilverstein
changed the base branch from
trunk
to
try/csm-e2e-chrome148-validation
|
Size Change: +115 B (0%) Total Size: 7.49 MB 📦 View Changed
|
adamsilverstein
changed the base branch from
try/csm-e2e-chrome148-validation
to
fix/external-image-server-sideload
adamsilverstein
force-pushed
the
try/csm-external-image-e2e
branch
from
428047b to
9fe9181
Compare
github-actions
Bot
removed
[Package] Block library
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Re-enables the
should upload external image to media librarye2e test (test/e2e/specs/editor/blocks/image.spec.js) and makes it CSM-agnostic.This is a CI-validation / staging PR that stacks two in-flight branches so the re-enabled test can run on the Chromium 148+ matrix with the server-side sideload present:
mediaSideloadFromUrl).Only
image.spec.js(the un-skip) and thebackport-changelogentry are unique to this branch; the other files are #79495's Chromium 148 / CSM e2e infrastructure.Why
#79495 skipped this test on Chromium 148+ because, under cross-origin isolation, the browser fetch of the remote image's bytes is blocked, so "Upload to Media Library" never finalized to a
/wp-content/uploads/URL. The skip comment promised CSM-aware coverage would be re-introduced once client-side external-image upload landed (#79407).#79409 moves the download/sideload to the server (
mediaSideloadFromUrl), so the upload no longer depends on the browser reading cross-origin bytes. With that in place the test passes whether or not the editor is isolated, so it can be un-skipped.How
test.skip( ... )back totest( ... ), drop the now-unnecessaryeslint-disable playwright/no-skipped-test, and replace the stale TODO with a note explaining the server-side sideload makes the flow CSM-agnostic./wp-content/uploads/assertion) already runs without one.Next steps
Once #79409 merges to trunk (and the Chromium 148 upgrade #79495 lands), this re-enablement rebases onto trunk and the staging PR can be closed.