Skip to content

Tiled Gallery: source Photon-domain check from block editor settings filter - #50426

Open
enejb wants to merge 8 commits into
trunkfrom
update/tiled-gallery-skip-photon-domain
Open

Tiled Gallery: source Photon-domain check from block editor settings filter#50426
enejb wants to merge 8 commits into
trunkfrom
update/tiled-gallery-skip-photon-domain

Conversation

@enejb

@enejb enejb commented Jul 10, 2026

Copy link
Copy Markdown
Member

Fixes JETPACK-1878

Proposed changes

The Tiled Gallery block — and Image Compare, which reuses Tiled Gallery's photonization — must serve images from the site's own files.wordpress.com-style host on VIP sites, instead of routing them through the public Photon domain (i0.wp.com), where VIP images are not accessible and return 403. That decision was made by isVIP(), which read the site plan from inline-script globals via a fragile window load listener that only populated when an Image Compare block happened to be present on the page. As a result, a plain Tiled Gallery on a VIP site never saw the plan and early-photonized its images (broken on VIP).

This PR makes the decision reliable and reusable:

  • Exposes a skipPhotonDomain flag in JetpackScriptData (Jetpack_Script_Data::set_admin_script_data()), sourced from the same signal as before: 'vip' === Jetpack_Plan::get()['product_slug'].
  • Adds a jetpack_skip_photon_domain filter so the decision can be overridden (defaults to true on VIP only).
  • Replaces isVIP() with a single exported skipPhotonDomain() that reads that flag; removes the inline jetpack_plan localize in render() and the window.load listener.
  • Keeps deprecated block versions on the external Photon domain. They must keep emitting the URLs they originally saved, so v2/v3/v4/v6/v8 no longer consult the site's setting — only the current version does.
  • Adds a deprecation for galleries saved before the setting was honoured, so VIP sites' existing galleries stay valid instead of showing "Block contains unexpected or invalid content".
  • Adds unit tests for the flag, for the src/srcSet it produces, and for the save → reload round trip across a setting change.

Default behavior is unchanged: skipPhotonDomain is false unless the plan slug is vip.

Why the delivery mechanism matters

skipPhotonDomain() feeds the blocks' save() output, so it is also evaluated when the editor regenerates markup to validate saved content while parsing the post. That happens before the editor stores hold any settings, so anything read from a store (an earlier iteration of this PR read a custom key added through block_editor_settings_all off core/editor) comes back as the default on that first pass and every saved gallery on a VIP site is flagged as invalid — thanks @coder-karen for catching this.

The value is therefore delivered as a plain global in the script data, which is printed inline before the editor scripts run and is available synchronously — the same channel as the neighbouring isSimpleSite() check that already influences this save() output.

Why existing galleries need a deprecation

For the same reason: photonized URLs live in the saved markup, so honouring the setting changes what save() emits, and galleries saved on a VIP site before this fix hold i0.wp.com URLs. Regenerating them now produces site-host URLs, which is a validation mismatch. The new deprecation reproduces the old output — same markup, external Photon domain — so those galleries parse as valid and are quietly re-serialized the next time they are saved. Attributes are untouched, so there is nothing to migrate.

Unlike the previous mechanism, the value is available for every block on every editor load — not only when an Image Compare block is present — so a plain Tiled Gallery on a VIP site is handled correctly. This generalizes the narrower VIP fix from #39941.

Related product discussion/links

Does this pull request change what data or activity we track or use?

No. The same plan signal is used; it is delivered through the script data instead of a per-block inline script.

Testing instructions

Because the fix concerns VIP sites, the simplest way to verify is to simulate a VIP plan on a normal Jetpack-connected site (e.g. Jurassic Ninja):

  1. Force the plan to VIP with a mu-plugin (sync-proof, intercepts every read):
    add_filter( 'pre_option_jetpack_active_plan', fn() => array( 'product_slug' => 'vip' ) );
  2. In the editor, add a Tiled Gallery with a few images — no Image Compare block needed (that's the case that was broken before). Save + reload.
  3. Confirm the image src/srcSet stay on the site's own host (https://<site>/wp-content/uploads/…) and are not rewritten to https://i0.wp.com/….
  4. Reload the post again and confirm there is no block-recovery / "this block contains unexpected or invalid content" prompt.
  5. In the console, window.JetpackScriptData.jetpack.flags.skipPhotonDomain should be true.
  6. Switch across the Tiled mosaic / Circles / Square tiles / Tiled columns styles; images should render correctly with no console errors.

Existing-content check (the case the deprecation covers):

  1. With the mu-plugin disabled, add and save a Tiled Gallery — its markup will use i0.wp.com, the way a VIP site's galleries were saved before this fix.
  2. Enable the mu-plugin, reload that post, and confirm the gallery is still valid with no recovery prompt. Saving it again moves the URLs onto the site's own host.

Baselines to confirm unchanged behavior:

  • On a non-VIP site, skipPhotonDomain is false and images use the standard Photon (i0.wp.com) path — unchanged.
  • On WordPress.com Simple sites, isSimpleSite() already forces the files.wordpress.com path, so behavior is unchanged there too.
  • Override check: add_filter( 'jetpack_skip_photon_domain', '__return_true' ); flips the flag to true on any site.

@enejb enejb added the [Status] Needs Review This PR is ready for review. label Jul 10, 2026
@enejb enejb self-assigned this Jul 10, 2026
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

  • To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack or WordPress.com Site Helper), and enable the update/tiled-gallery-skip-photon-domain branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack update/tiled-gallery-skip-photon-domain
bin/jetpack-downloader test jetpack-mu-wpcom-plugin update/tiled-gallery-skip-photon-domain

Interested in more tips and information?

  • In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
  • Read more about our development workflow here: PCYsg-eg0-p2
  • Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

@github-actions github-actions Bot added [Block] Tiled Gallery [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ labels Jul 10, 2026
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


Follow this PR Review Process:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!


Jetpack plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

@jp-launch-control

jp-launch-control Bot commented Jul 10, 2026

Copy link
Copy Markdown

Code Coverage Summary

Coverage changed in 23 files. Only the first 5 are listed here.

File Coverage Δ% Δ Uncovered
projects/plugins/jetpack/extensions/blocks/tiled-gallery/tiled-gallery.php 69/429 (16.08%) -0.39% 0 💚
projects/plugins/jetpack/src/class-jetpack-script-data.php 10/10 (100.00%) 0.00% 0 💚
projects/plugins/jetpack/extensions/shared/block-styles.js 10/10 (100.00%) 10.00% -1 💚
projects/plugins/jetpack/extensions/blocks/tiled-gallery/deprecated/v1/index.js 2/2 (100.00%) 100.00% -2 💚
projects/plugins/jetpack/extensions/blocks/tiled-gallery/deprecated/v2/index.js 2/2 (100.00%) 100.00% -2 💚

4 files are newly checked for coverage.

File Coverage
projects/plugins/jetpack/extensions/blocks/tiled-gallery/deprecated/v9/save.jsx 5/6 (83.33%) 💚
projects/plugins/jetpack/extensions/blocks/tiled-gallery/deprecated/v9/attributes.js 0/0 (—%) 🤷

Full summary · PHP report · JS report

Copilot AI 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.

Pull request overview

This PR fixes VIP-site image breakage in the Tiled Gallery (and Image Compare, which reuses its photonization logic) by making the “skip external Photon domain” decision reliably available to block editor code via block editor settings, rather than via a fragile window.load + global state approach.

Changes:

  • Expose a skip_photon_domain boolean through the block_editor_settings_all filter, defaulting to true only for VIP plans and allowing overrides via jetpack_skip_photon_domain.
  • Replace the per-block/version VIP detection with a single shared skipPhotonDomain() helper that reads from core/editor’s editor settings.
  • Remove legacy inline localization (jetpack_plan) and deduplicate deprecated block versions to import the shared helper.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
projects/plugins/jetpack/extensions/blocks/tiled-gallery/utils/index.js Removes fragile load-time plan sniffing and adds skipPhotonDomain() sourced from core/editor settings.
projects/plugins/jetpack/extensions/blocks/tiled-gallery/tiled-gallery.php Adds block_editor_settings_all filter to inject skip_photon_domain and introduces jetpack_skip_photon_domain override filter.
projects/plugins/jetpack/extensions/blocks/tiled-gallery/deprecated/v6/utils/index.js Drops local VIP check and imports shared skipPhotonDomain() for consistent behavior.
projects/plugins/jetpack/extensions/blocks/tiled-gallery/deprecated/v4/utils/index.js Drops local VIP check and imports shared skipPhotonDomain() for consistent behavior.
projects/plugins/jetpack/extensions/blocks/tiled-gallery/deprecated/v3/utils/index.js Drops local VIP check and imports shared skipPhotonDomain() for consistent behavior.
projects/plugins/jetpack/extensions/blocks/tiled-gallery/deprecated/v2/utils/index.js Drops local VIP check and imports shared skipPhotonDomain() for consistent behavior.
projects/plugins/jetpack/changelog/update-tiled-gallery-skip-photon-domain Adds a Jetpack plugin changelog entry documenting the behavior change and new filter.

@coder-karen coder-karen 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.

Thanks for looking into this!

I can confirm everything works as expected following the testing steps.

However, I do see a block validation issue after just refreshing the page, after the post is saved (the saved content includes photonized URLs).

I believe the issue is that editor settings are runtime editor state, not block attributes. Since block validation operates on parsed block attributes and regenerated markup, code that runs during parsing or validation shouldn't depend on editor settings, which is my understanding based on the information here.

Could a possibility be to keep the same capability-based check, but PHP side? Then just change the delivery by extending Jetpack_Editor_Initial_State?

@enejb

enejb commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

@coder-karen good catch — confirmed, and the diagnosis was exactly right. skipPhotonDomain() feeds save(), which the editor re-runs while parsing saved content for validation, and that happens before the editor stores hold any settings. So the core/editor read came back undefined on that pass, the regenerated src went to i0.wp.com while the post body had the site host, and every saved gallery got flagged.

Went with your suggestion: the check stays in PHP on the same plan signal (with the jetpack_skip_photon_domain override) and is now delivered in Jetpack_Editor_Initial_State, which is printed inline before the editor scripts and readable synchronously — the same channel as the isSimpleSite() check right next to it, which already affects this save output.

Verified on a JN site with the plan forced to vip, same post and same site, only the plugin code changing:

  • pre-fix: block isValid: false, "Block contains unexpected or invalid content / Attempt recovery", console Block validation: Expected attribute src of value https://i0.wp.com/…?ssl=1, saw https://<site>/wp-content/uploads/…
  • fixed: valid, no banner, no validation messages
  • non-VIP baseline: skip_photon_domain is false, images use the Photon path, block valid — unchanged
  • add_filter( 'jetpack_skip_photon_domain', '__return_true' ) on a free plan flips it as expected
  • frontend still serves …/wp-content/uploads/…?strip=info&w=600, so the PHP srcset builder is fine after dropping the old wp_localize_script

Also added unit tests that set only the initial-state global, so a store-based read would fail them. PR description and testing instructions are updated with the reload / no-recovery-prompt step.

@enejb
enejb force-pushed the update/tiled-gallery-skip-photon-domain branch from e930611 to e922bc6 Compare July 31, 2026 21:48
@enejb enejb added this to the 16.1 milestone Jul 31, 2026

@coder-karen coder-karen 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.

Thanks for the follow-up!

The scenario resulting in block validation issues now no longer does -- though I notice there is still one scenario that does: a TG block added on a VIP site (or in my case a mock VIP site using the mu-plugin) before the branch is applied. Once the branch is applied and the post / page refreshed, the block validation issue is visible.

Ultimately the 'save' logic is still different (checking skipPhotonDomain vs true == isVIP) in the deprecations. Shouldn't only the current version be using skipPhotonDomain, and each deprecation be still forcing the old Photon path, perhaps then needing a new deprecation? 🤔

* @return {boolean} True when the external Photon domain should be skipped.
*/
export function skipPhotonDomain() {
return true === getJetpackData()?.jetpack?.skip_photon_domain;

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.

getJetpackData is deprecated, and getScriptData is recommended now:

* @deprecated Use the consolidated initial state using `getScriptData` from `@automattic/jetpack-script-data` instead. Feel free to extend it if needed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Switched to getScriptData. The flag now ships in JetpackScriptData via Jetpack_Script_Data::set_admin_script_data() and is read as getScriptData()?.jetpack?.flags?.skipPhotonDomain, matching how showJetpackBranding is read. Still a synchronous inline global, so it is available while the post is parsed — which is the constraint that ruled out editor settings.

@enejb

enejb commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@coder-karen both good catches — and your instinct on the deprecations was the fix for the remaining scenario, so thanks for pushing on it.

Deprecations. You're right that they shouldn't have been following the site's setting: they exist to reproduce markup that was already saved, and content saved under them holds external Photon URLs. v2/v3/v4/v6/v8 now always use the Photon domain, and only the current version consults the setting. (In passing, that also means the old private isVIP() copies were dead weight rather than something to share — in the editor they never saw a plan at parse time, so they always resolved to the Photon path. Removing the check outright is what actually preserves their behaviour.)

The remaining validation scenario. Fixed, and it is the deprecation change that fixes it — a gallery saved on a (mock) VIP site before the branch is applied now stays valid after applying it, no recovery prompt, and the block is quietly re-serialized onto the site's own host the next time it is saved.

I also added a dedicated deprecation mirroring the current markup with the Photon domain forced. Worth flagging why, since it is arguably redundant: I tested with it removed, and that content still validates — but only against v8, and only because block validation forgives the stray whitespace node in v8's wrapper (…is-style-rectangular"> <div vs …"><div). Their save output is otherwise identical. Leaning on that felt too incidental, so the new deprecation matches the current shape exactly and a unit test pins it to save.jsx so the two can't drift apart. Happy to drop it if you'd rather keep the deprecation list shorter — it's a two-line revert.

Verified on a JN site with the plan forced to vip, same posts and same site, only the plugin code changing:

  • gallery saved before the branch (Photon URLs) → valid after the branch, no recovery prompt, console confirms a deprecation matched for jetpack/tiled-gallery
  • gallery saved with the branch (site-host URLs) → valid, no deprecation needed
  • non-VIP → skipPhotonDomain is false, images on the Photon path, valid — unchanged
  • the PHP flag itself, against a real site: viptrue, jetpack_freefalse, jetpack_skip_photon_domain override on a free plan → true

688 JS tests pass, including new coverage for the save → reload round trip across a setting change. Two notes on limits: the new PHPUnit cases in Jetpack_Script_Data_Test weren't run locally (that suite needs the Docker WP environment), which is why I checked those three states directly against a live site instead — CI will run them. And one case is deliberately not covered: a site whose plan actually flips VIP → non-VIP will invalidate galleries in the other direction. That is pre-existing for any environment-dependent save() (isSimpleSite() included) and would need a second deprecation for the inverse; happy to add it if you think it's worth it, but it seemed like speculative coverage.

PR description and testing instructions updated, including steps for the existing-content case.

@enejb

enejb commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Correction to my last comment: I said the new deprecation was arguably redundant because v8 also matched the pre-fix markup, and offered to drop it. That was wrong on both counts.

Re-tested each deprecation individually against pre-fix markup:

  • the accidental matcher is v6, not v8 — v8's gallery-image/save emits role/tabindex/aria-label and data-custom-link, so it doesn't match at all
  • v6 only covers plain galleries. For linkTo: 'custom' the new deprecation is the only match — dropping it makes every pre-fix VIP gallery with custom links invalid (verified: isValid true with it, false without)

So it stays. Three follow-ups from that:

  • its attributes/supports are now frozen local snapshots instead of live block.json reads, matching the v8 convention — a deprecation that tracks current metadata stops reproducing what's in the database
  • the docblock said "keep this file in step with save.jsx", which inverts the contract. It now says the opposite: never edit it; add a new deprecation when the markup changes
  • I dropped the test that asserted the deprecation's output equals the current save() — it would have gone red on any legitimate markup change and pushed whoever hit it into making exactly the edit that breaks old content. Replaced with a custom-link case that fails if the deprecation is removed, which is the guard I actually wanted

Also in this push: skipPhotonDomain added to the jetpack.flags type in shared-extension-utils/declarations.d.ts (TS consumers would have errored on it), a redundant ternary removed in layout/index.jsx, and a note in the PHP docblock on why the plan slug is the signal rather than Host::is_vip_site() — so the next reader doesn't switch it and silently change which sites need a deprecation.

@github-actions github-actions Bot added the RNA label Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Block] Tiled Gallery [JS Package] Shared Extension Utils [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ RNA [Status] Needs Review This PR is ready for review. [Tests] Includes Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants