Jetpack: Carousel: images not displayed when a Tiled Gallery uses external images

Created on 14 Jan 2021  路  11Comments  路  Source: Automattic/jetpack

Steps to reproduce the behavior

  1. Add a tiled gallery to post/page
  2. Publish post/page and click on the image.

What I expected to happen

Expected gallery to open in a carousel and images to appear.

What actually happened

  1. Carousel opens but the image appears blank.
  2. Console displays the following error.

Context

26991480-hc

Browser / OS version

Tested in Firefox 84 & Chrome 87 / Mac OS Big Sir

Is this specific to the applied theme? Which one?

Tested on Storefront, TwentyTwenty, Varia child themes. Does not appear to be theme specific.

Does this happen on simple or atomic sites or both?

Both Simple and Atomic

Is there any console output or error text?

Uncaught TypeError: e(...).data(...) is undefined
    originalDimensions https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    bestFit https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    fitInfo https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    updateSlidePositions https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    selectSlide https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    selectSlideAtIndex https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    open https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jQuery 28
    open https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jQuery 2
    open https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    <anonymous> https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jQuery 2
jetpack-carousel.min.js:3:19301
    originalDimensions https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    bestFit https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    fitInfo https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    updateSlidePositions https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    selectSlide https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    selectSlideAtIndex https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    open https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jQuery 28
    open https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jQuery 2
    open https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jp_carousel https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    <anonymous> https://c0.wp.com/p/jetpack/9.3/_inc/build/carousel/jetpack-carousel.min.js:3
    jQuery 2

Level of impact (Does it block purchases? Does it affect more than just one site?)

Seems to affect all users with a gallery carousel on their site.

Screenshot / Video: If applicable, add screenshots to help explain your problem.

wpcarousel

Carousel [Block] Tiled Gallery [Pri] Normal [Type] Bug [Type] Happiness Request

Most helpful comment

Repro here: https://test79088093.wordpress.com/portfolio/ (using a page layout / pattern content)

Narrowed this down to this $attachments array being empty. The jQuery error we see above results when data-orig-size=.. doesn't get set along with other image attributes. These are only set when attachments are found.

The $selected_images array is not empty and looks correct in my example.

The images in this example are not present in the media library. Likely because it's example content that uses image urls for the content from the pattern source site.

For example, in this test post I used images directly from the media library and everything works fine: https://test79088093.wordpress.com/19-2/

I'm not sure where the fix belongs, @jeherve - do you think Jetpack needs to handle the hotlink usecase? It's probably out of scope but we might be able to at least get it to gracefully handle the situations (loop on selected images instead of attachments and add defaults where we can).

cc @andrewserong / @apeatling as maybe ya'll have a better idea / and or want to take this issue on. e.g. copying images over to the media library and using those in the pattern content instead. (not easy I know)

All 11 comments

I do not believe this is the same issue but will reference it here if it is helpful to track down the cause.

https://github.com/Automattic/jetpack/issues/11191

This issue looks like it's definitely related to https://github.com/Automattic/jetpack/issues/11191 in some way.

Unfortunately I am unable to reproduce this exact error. However using similar images in terms of file size I was able to come across some issues with the Bulk Uploader but I am not able to consistently reproduce this bug either.

Due to the images referenced in the HC of the issue description, one thing that stands out is the size of the images as they're extremely large but again all attempts to reproduce the original issue have failed. I wonder if this issue would persist if the images we downsized, compressed and reuploaded?

Internal repro here: p1muIj-837-p2 no luck reproing from scratch yet

This has come up in the public forums here - the console shows these errors: https://d.pr/i/1F7QA7 (same as Automattic/jetpack#11191)

Something I also identified is that the image being displayed in the gallery (an ordinary gallery block) - https://tepe68blog3.wordpress.com/2020/12/22/rosabell-laurenti-sellers/ isn't present into the media gallery.

Repro here: https://test79088093.wordpress.com/portfolio/ (using a page layout / pattern content)

Narrowed this down to this $attachments array being empty. The jQuery error we see above results when data-orig-size=.. doesn't get set along with other image attributes. These are only set when attachments are found.

The $selected_images array is not empty and looks correct in my example.

The images in this example are not present in the media library. Likely because it's example content that uses image urls for the content from the pattern source site.

For example, in this test post I used images directly from the media library and everything works fine: https://test79088093.wordpress.com/19-2/

I'm not sure where the fix belongs, @jeherve - do you think Jetpack needs to handle the hotlink usecase? It's probably out of scope but we might be able to at least get it to gracefully handle the situations (loop on selected images instead of attachments and add defaults where we can).

cc @andrewserong / @apeatling as maybe ya'll have a better idea / and or want to take this issue on. e.g. copying images over to the media library and using those in the pattern content instead. (not easy I know)

That's an interesting one. Jetpack definitely should handle this use-case better, it would be worth moving this issue to the Jetpack repo. I'l do that now.

The images in this example are not present in the media library

Do you think you could add some steps to follow to create a tiled gallery, or a core gallery, using images that are not in the Media Library? I did not know that was possible :)

Do you think you could add some steps to follow to create a tiled gallery, or a core gallery, using images that are not in the Media Library? I did not know that was possible :)

If a page template is used in WordPress.com, the images are not added to the Media Library.

Screen Capture on 2021-01-16 at 14-57-10

Alternatively, if I copy a tiled gallery block from the editor of one site and paste that into another site, the images are also not added to the Media Library.

If a page template is used in WordPress.com, the images are not added to the Media Library.

Ah, that's interesting. That seems like a bug. I believe images were side-loaded since https://github.com/Automattic/wp-calypso/pull/34823 and https://github.com/Automattic/wp-calypso/pull/34839, so I would recommend creating an issue in the Calypso repo about this.

if I copy a tiled gallery block from the editor of one site and paste that into another site, the images are also not added to the Media Library.

That's another interesting edge-case. Thank you, that should help us reproducing and when testing a fix.

@jeherve we've decided that it's better UX and better use of customers hosting space if we don't populate the media gallery with demo-images, as we expect customers to anyway replace those images with their own.

That said, I don't see why the Jetpack Carousel shouldn't work for external images and this definitely is still a bug. Customers can add external images to the image block just fine manually as well.

@apeatling added to View backlog for your consideration, let us know if you'd prefer not to have this. :-)

cc @sgomes in case this is after recent carousel jQuery refactoring?

@simison : The first stage of the carousel refactoring is only live in wpcom, and hasn't made its way into Jetpack yet. So if the issue is showing up in atomic, it's likely unrelated.

Per the discussion above, the issue appears to be that certain data attributes are expected, but are not being populated for these images.

Marked this issue in a note as part of a bigger carousel rethink.

Was this page helpful?
0 / 5 - 0 ratings