Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing images in amp-carousel added by jetpack #5377

Closed
rsmith4321 opened this issue Sep 14, 2020 · 7 comments
Closed

Missing images in amp-carousel added by jetpack #5377

rsmith4321 opened this issue Sep 14, 2020 · 7 comments
Labels
Invalid Upstream Bug Requires an upstream change from WordPress, Gutenberg, or another dependency.

Comments

@rsmith4321
Copy link

Bug Description

If I manually add an amp-carousel using an html block, all images load correctly. However, if I add a carousel using the jetpack slideshow block, which is converted by the amp plugin into an amp-carousel, on the first page load there is always missing images. If the page is refreshed the images appear. This is an issue I've had for a long time.

Expected Behaviour

All images should appear after cache is cleared and page first loads.

Steps to reproduce

Load the page here https://www.ryansmithphotography.com for the first time or after clearing cache.

Screenshots

https://drive.google.com/file/d/1go9FYGGTugRTyUlqvqKMNLTx636hxBBJ/view?usp=sharing

Additional context

  • WordPress version: 5.5.1
  • Plugin version: 2.0.1
  • Gutenberg plugin version (if applicable):
  • AMP plugin template mode: Native
  • PHP version: 7.4.1
  • OS: Mac and Android
  • Browser: [e.g. chrome, safari] Chrome
  • Device: [e.g. iPhone6] Mac and Galaxy S20

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation brief

QA testing instructions

Demo

Changelog entry

@westonruter
Copy link
Member

@rsmith4321 The screenshots aren't accessible.

And actually, the AMP plugin doesn't do any conversion of the Slideshow block. It is emitted as an <amp-carousel> directly from Jetpack: https://github.com/Automattic/jetpack/blob/40e2c3f49324fa9fefa54c31674ab2ef27451d82/extensions/blocks/slideshow/slideshow.php#L81-L106

Therefore, the bug is either with Jetpack or with AMP itself. I see you've reported this to AMP as well: ampproject/amphtml#27036 (comment)

@westonruter
Copy link
Member

I wonder if it has anything to do with server-side rendering.

Could you try adding this plugin: https://gist.github.com/westonruter/8d52c0b807e6dfbbdf2219622d0f4a7e

That will disable SSR, and then you can force it on with ?amp_ssr=true or off with ?amp_ssr=false.

@rsmith4321
Copy link
Author

rsmith4321 commented Sep 15, 2020 via email

@westonruter
Copy link
Member

westonruter commented Sep 15, 2020

SSR doesn't create a cache. It just reduces the amount of work that the AMP runtime JS has to do once the page loads. Apparently this is reducing too much of the work that the runtime has to do, as it's not going so far as to render that image.

Please leave that "Disable SSR" plugin active so that the AMP team can compare the page with and without SSR.

@rsmith4321
Copy link
Author

rsmith4321 commented Sep 16, 2020 via email

@rsmith4321
Copy link
Author

rsmith4321 commented Sep 16, 2020 via email

@westonruter
Copy link
Member

I can reproduce it, and it is frustratingly sporadic. I took a screencast of reproducing the issue after several failed attempts at reloading: https://youtu.be/nBtjBtai8s0

Unfortunately, this doesn't appear to have anything to do with the AMP plugin, so I'll follow up on the AMP core issue.

@westonruter westonruter added Invalid Upstream Bug Requires an upstream change from WordPress, Gutenberg, or another dependency. labels Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Invalid Upstream Bug Requires an upstream change from WordPress, Gutenberg, or another dependency.
Projects
None yet
Development

No branches or pull requests

2 participants