Gallery Block: images not rendered when the site is private atomic

[adamziel]

Steps to reproduce

1. Create a new private site (through the variant group of ATPrivacy A/B test)
2. Go Atomic
3. Upload a new media file to make sure we're using something that hasn't been cached by Photon yet
4. Create a tiled gallery with that new media file
5. Visit the site and confirm it's not displayed

Expected result

Properly working tiled gallery

Description Tiled gallery module images are not rendered when the site is Private and Atomic. Jetpack always “photonizes” these URLs, even when Photon module is disabled:

https://github.com/Automattic/jetpack/blob/e07addf220d02226f04d874b1b565ceb9f502839/modules/tiled-gallery/tiled-gallery/tiled-gallery-item.php#L35

Ultimately, when Photon-Calypso project lands, it will be possible to build on top of it and add support for the images loaded through the site itself (adding media proxy endpoint to wpcomsh in order to use authenticated Photon requests). We're not there yet though.

For now, the solution would involve a short PR to Automattic/wpcomsh. Specifically, plugging into one of the filters in jetpack_photon_url function and completely disabling photon for the Gallery. We could even backfill the thumbnail URLs with the closest core thumbnail (Automattic/wpcomsh/blob/master/private-site/private-site.php#L317) (or even create one just for the gallery).

[andrewserong]

I made a start looking into this today on a test Atomic site. I haven't been able to successfully skip the jetpack_photon_url call so far (I'm trying variants of the following:)

function force_skip_jetpack_photon_url( $skip, $image_url ) {
    return true;
}
add_filter( 'jetpack_photon_skip_for_url', '\Private_Site\force_skip_jetpack_photon_url', 10, 2 );

I'll do more digging tomorrow, not sure if it's that I need to use a different priority, but from a first glance, it does look like the rendered HTML ends up getting saved to the post content with the Photon URL as the source (if I'm not mistaken), so I'm not sure what the implications are for tiled galleries created while Private, and then a user switches their site to Public. Would they need to update the block for Photon urls to start working? 🤔I'll do more digging tomorrow!

[adamziel]

Interesting! If the photon URL is stored in post content then this issue gets more complicated. I know Photon hooks into some filters to replace the regular URLs with photon URLs on the flight here:

https://github.com/Automattic/jetpack/blob/d38a6554c8eb22b3284f5e8c750118ea5e201379/class.photon.php#L231

Maybe we should perform a reverse operation here? So filter the content to replace all the photon URLs with regular ones?

[simison]

Correct, Photon cannot be skipped for Tiled gallery (the block nor the legacy shortcode version). It requires Photon for cropping and resizing images on the fly the way that self-hosted WP cannot really do.

Elaborated more on reasons behind this architecture on Slack p1585067715159000-slack-jetpack-crew

[simison]

Maybe we should perform a reverse operation here? So filter the content to replace all the photon URLs with regular ones?

@adamziel do you mean that we'd replace Photonized URL with the original media-library file URL? The original file in media-library would be wrong size, compared to the one the block needs.

[adamziel]

@simison The ultimate fix would involve implementing the same media proxy we have in public-api. With that in place, the browser would request the media proxy endpoint instead of i[0-9].wp.com, the media proxy would request photon with authentication token. One pre-requisite for that is photon support for authenticated requests. It is in progress but not there yet, we also don't have a clear estimate for merging the patch - see the discussion here: pMz3w-b58-p2

As a workaround, I'm proposing using an approximately similar thumbnail generated by local WP. The specific size would differ, but the gallery would keep working and with some luck would not look terrible. An even better workaround would be using local WP to create a set of gallery-specific thumbnails and using them instead of going through photon.

[andrewserong]

I did some more digging today, but unfortunately it looks pretty tricky to handle this in wpcomsh to me. I was confused at first, because I was looking at the legacy Tiled Gallery in the Jetpack plugin instead of the Tiled Gallery block. The block appears to generate the Photonized img src via JavaScript (which would explain why the filters weren't working). So even if we could update the post content after the fact to point to the media library URLs, we still wouldn't have the images working within the editor.

Unfortunately I can't think of a clean way to do the workaround from the wpcomsh end of things, and I'm not quite sure what the best approach would be to change how the block behaves in Jetpack. Any ideas for how we could handle this in Jetpack, or does this veer too far from our ideal behaviour of authenticated Photon requests? (Happy to keep exploring in and around the other tasks this fortnight).

[adamziel]

@andrewserong this is tricky indeed!

I don't think we should change what's getting stored. My understanding is that the block wraps regular URLs in photon and they're stored as such. Now, imagine the site is public and the user makes it private. I don't think we should be updating all the stored posts and pages in response to that change - filtering the post content before echo-ing sounds much better. The filter would be only enabled when the site is private.

When the media proxy is available/possible, the filter would "wrap" photon URLs in proxy URLs. Until then, it could replace the photon URLs with references to local thumbnails.

Would that work? What do you think?

[simison]

filtering the post content before echo-ing sounds much better.

Would that include only frontend of the site? If so, that sounds good. If also editor (so basically via the posts API), that might be problematic since you need to keep the editor content identical to block's save() output, or otherwise the block will invalidate.

Until then, it could replace the photon URLs with references to local thumbnails.

That would completely break the gallery layout and be kinda pointless fix, since it would just break in other way instead; Photon not only resizes the images, but it also crops them in different shapes than the original file was.

Ultimately, when Photon-Calypso project lands, it will be possible to build on top of it and add support for the images loaded through the site itself

How does the timeline look?

[adamziel]

Would that include only frontend of the site? If so, that sounds good. If also editor (so basically via the posts API), that might be problematic since you need to keep the editor content identical to block's save() output, or otherwise the block will invalidate.

That's a very good question - I guess it depends whether or not the editor uses photon-ed thumbnails when editing the page. If it does, then we're in a territory where a Jetpack PR is likely required.

That would completely break the gallery layout and be kinda pointless fix, since it would just break in other way instead; Photon not only resizes the images, but it also crops them in different shapes than the original file was.

Is there an easy way to do it locally?

How does the timeline look?

We agreed on the approach and the code is pretty much there after a few rounds of feedback so it should be good to go? The timeline is unclear though, see the following comment: pMz3w-b58-p2#comment-78076 - also CCing @johnHackworth

[adamziel]

Also the logical next step would be evaluating if there are other places in Jetpack where photon is "baked in" even when the module is disabled.

[simison]

I guess it depends whether or not the editor uses photon-ed thumbnails when editing the page.

It does use; the editor side is basically identical with the frontend side.

Is there an easy way to do it locally?

Not that I know of. Would've definitely gone ahead that way otherwise. Image sizes and shapes are recalculated each time you add, remove, or re-order images.

Simple and VIP sites don't use i[0-9].wp.com Photon servers and use .files.wordpress.com files instead, which seems to be an instance of Photon as well. I wonder if Atomic sites could also use those and if implementing privacy for those URLs is easier?

https://github.com/Automattic/jetpack/blob/a9c70935380fdcb7deee882c608083f52d865b9d/extensions/blocks/tiled-gallery/utils/index.js#L121

https://github.com/Automattic/jetpack/blob/a9c70935380fdcb7deee882c608083f52d865b9d/extensions/blocks/tiled-gallery/utils/index.js#L53-L54

[simison]

Also the logical next step would be evaluating if there are other places in Jetpack where photon is "baked in" even when the module is disabled.

@jeherve @kraftbj would know if someone. :-)

[kraftbj]

Tiled Galleries and Related Posts (for the thumbnail) are the spots off the top of my head.

[jeherve]

The Top Posts Widget and the Display Posts widgets also rely on Photon, and we use Photon URLs when using Content Options in some a8c themes to display images on archive pages.

[andrewserong]

Apologies, I didn't get much further with this one during my maintenance rotation. I'm unassigning myself now, but it's still in our maintenance backlog for anyone to pick up.

[sirreal]

This seems to be blocked on some photon-side work in pMz3w-b58-p2 that @adamziel is working on with other teams.

Is that accurate?

[adamziel]

I handed off that work to @johnHackworth and the create focus, although it's mostly done - it's just waiting for deployment (or final-ish feedback).

[ockham]

This is still blocked, see pMz3w-b58-p2#comment-79064.

[happychait]

Ran into this issue with "Coming Soon" mode while trying to add a Tiled Gallery block. The editor wouldn't show the gallery images.

21566134-hc

[tanjoymor]

Had another customer run into this 21748691-hc

[rinazrina]

Another report: 21734563-hc

[happychait]

20128576-hc

[happychait]

21839948-hc

[rachelwinspear]

Found issue in Quick Start session: 3067207-zen

[jamiepalatnik]

Reported in 22183595-hc

[jackiejade]

Reported in 21436641-hc

[iamAfsana]

Ran into this same issue with "Coming Soon" mode while trying to add a Tiled Gallery block. The editor wouldn't show the gallery images. #3160629-zen

[happychait]

22854706-hc

[joweber123]

Reported in 23043625-hc

[scosgro]

Another WordPress.com user report in 23740415-hc

[jartes]

Reported in a private AT site 3294794-zen

[ash1eygrace]

Another report pushed to 3291416-zen - Had them install a Coming Soon plugin as a workaround.

[sdixon194]

Another case in 24155539-hc

[sourourn]

Another report: #24185808-hc

[sdixon194]

Another case 24234017-hc

[jartes]

Another case 24325736-hc

[adamziel]

@ramonjd I tagged you in so that you're notified about new comments here

[lakellie]

Another report 3424301-zen

[ramonjd]

I've left another note over at pMz3w-b58-p2#comment-83240

[susanjanec]

Another case - 25322363-hc - provided workaround to set site to Public, and add SeedProd coming soon plugin.

[nerdysandy]

Reported on #25672569-hc

[geekinthegirl]

Another potential case here: 24865600-hc - I asked them to launch their site to test this, and they went quiet, but dropping it here just in case.

[katiebethbrown]

Another report - 27686765-hc

Suggested using Coming Soon mode instead.

[Agepeuve]

Reported in 27779051-HC

[Addison-Stavlo]

Lowered priority due to smaller % of users experiencing this issue after Coming Soon v2 changes. Fix continued to be stalled pMz3w-b58-p2#comment-83240

[rinazrina]

Another one: #3749605-zen

[tanjoymor]

This same issue occurred on a site using the Classic Editor with a gallery: #27874150-hc

[happychait]

24413916-hc

[kwight]

@adamzelinski I'm not sure if I'm missing something here – I can't seem to replicate (new site, transfer to Atomic, add tiled gallery with new images):

"Coming soon" is the same as "private" now, is that correct?..

[ramonjd]

"Coming soon" is the same as "private" now, is that correct?..

👋 Coming soon used to be equivalent to private mode beneath the chassis.

Last year we switched Coming soon "mode" to public not-indexed. So the resources are still public, but the pages/posts are "hidden" behind a Coming Soon site.

So I think it's strictly related to a site that's been explicitly set to private in the settings.