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

Carousel: images not displayed when a Tiled Gallery uses external images #18362

Open
joweber123 opened this issue Jan 14, 2021 · 15 comments
Open
Labels
[Block] Tiled Gallery Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended

Comments

@joweber123
Copy link
Contributor

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

@joweber123
Copy link
Contributor Author

joweber123 commented Jan 14, 2021

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

#11191

@tjcafferkey
Copy link
Contributor

tjcafferkey commented Jan 14, 2021

This issue looks like it's definitely related to #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?

@cathymcbride
Copy link

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

@aleone89
Copy link

This has come up in the public forums here - the console shows these errors: https://d.pr/i/1F7QA7 (same as #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.

@lsl
Copy link
Contributor

lsl commented Jan 15, 2021

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)

@jeherve
Copy link
Member

jeherve commented Jan 15, 2021

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 :)

@jeherve jeherve transferred this issue from Automattic/wp-calypso Jan 15, 2021
@matticbot matticbot added the Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" label Jan 15, 2021
@jeherve jeherve added [Block] Tiled Gallery [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Type] Bug When a feature is broken and / or not performing as intended [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Pri] Normal labels Jan 15, 2021
@jeherve jeherve changed the title Gallery Carousel Does Not Display Images Carousel: images not displayed when a Tiled Gallery uses external images Jan 15, 2021
@joweber123
Copy link
Contributor Author

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.

@jeherve
Copy link
Member

jeherve commented Jan 18, 2021

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 Automattic/wp-calypso#34823 and Automattic/wp-calypso#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 jeherve removed the [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! label Jan 18, 2021
@simison
Copy link
Member

simison commented Feb 23, 2021

@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?

@sgomes
Copy link
Contributor

sgomes commented Feb 23, 2021

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

@apeatling
Copy link
Member

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

@yansern
Copy link
Contributor

yansern commented Sep 8, 2021

This is no longer replicable. With all the rework being done in Carousel I think this is no longer and issue.
See commit history: https://github.com/Automattic/jetpack/commits/master/projects/plugins/jetpack/modules/carousel/jetpack-carousel.js

@yansern yansern closed this as completed Sep 8, 2021
@aleone89
Copy link

aleone89 commented Jun 9, 2023

Hey there,

I've just come across this bug twice on two simple sites. The first incident can be found here:

https://wordpress.com/forums/topic/subscribers-137/?view=all (using images loaded into the sites media library)

The second can be found here:

https://wordpress.com/forums/topic/bug-in-gallery-widget/ (using external images)

When clicked, the console displays these two errors in both of these sites here:

Uncaught TypeError: Cannot read properties of undefined (reading 'closest') at Object.closest (??-eJyNUNFuAjEM+6GVaNNAe5n2KajXBmhpmlOTHvv8BcQdE0JsfapjO24Np9EFropVIQtEnFLA8XuV5QWMSjWUHlHO3IEnbOkidVlWlOojlY9GuMG334I5YSx9n6qAHpDQEdZ+ttzQvZ66my0ZdfTheMVAzBW2FgxDTyVC8I27YFl08+DRM/5Ye51tJ6yRG/iuTF41hUW940ZWVhKFoXA4CkwJT09illaMD0zu1tJ/PORFsdnNabN4eWKav+0uOXfQfF/0+br5WL9v3uzkH2snvrk=:722:1408) at H (??-eJyNUNFuAjEM+6GVaNNAe5n2KajXBmhpmlOTHvv8BcQdE0JsfapjO24Np9EFropVIQtEnFLA8XuV5QWMSjWUHlHO3IEnbOkidVlWlOojlY9GuMG334I5YSx9n6qAHpDQEdZ+ttzQvZ66my0ZdfTheMVAzBW2FgxDTyVC8I27YFl08+DRM/5Ye51tJ6yRG/iuTF41hUW940ZWVhKFoXA4CkwJT09illaMD0zu1tJ/PORFsdnNabN4eWKav+0uOXfQfF/0+br5WL9v3uzkH2snvrk=:722:16560) at C (??-eJyNUNFuAjEM+6GVaNNAe5n2KajXBmhpmlOTHvv8BcQdE0JsfapjO24Np9EFropVIQtEnFLA8XuV5QWMSjWUHlHO3IEnbOkidVlWlOojlY9GuMG334I5YSx9n6qAHpDQEdZ+ttzQvZ66my0ZdfTheMVAzBW2FgxDTyVC8I27YFl08+DRM/5Ye51tJ6yRG/iuTF41hUW940ZWVhKFoXA4CkwJT09illaMD0zu1tJ/PORFsdnNabN4eWKav+0uOXfQfF/0+br5WL9v3uzkH2snvrk=:722:21103) at T.a.onload (??-eJyNUNFuAjEM+6GVaNNAe5n2KajXBmhpmlOTHvv8BcQdE0JsfapjO24Np9EFropVIQtEnFLA8XuV5QWMSjWUHlHO3IEnbOkidVlWlOojlY9GuMG334I5YSx9n6qAHpDQEdZ+ttzQvZ66my0ZdfTheMVAzBW2FgxDTyVC8I27YFl08+DRM/5Ye51tJ6yRG/iuTF41hUW940ZWVhKFoXA4CkwJT09illaMD0zu1tJ/PORFsdnNabN4eWKav+0uOXfQfF/0+br5WL9v3uzkH2snvrk=:722:20263)

@aleone89 aleone89 reopened this Jun 9, 2023
@sophiegyo
Copy link

Just to note, the gallery mentioned in https://wordpress.com/forums/topic/bug-in-gallery-widget/ is actually a regular gallery, not a tiled gallery specifically.

@airdrummer
Copy link
Contributor

airdrummer commented Jun 23, 2023

i have the same issue when enabling image load speedup: the gallery display is blank. on inspection, the carousel links are missing parameters:

https://i0.wp.com/cardinalglen.org/wp-content/uploads/2023/06/IMG_1651.gif

404s, but the inspector also shows:

https://i0.wp.com/cardinalglen.org/wp-content/uploads/2023/06/IMG_1651.gif?w=394&h=289&ssl=1

which does load…see https://cardinalglen.org/buckbrosbreakfast/

work-around: disable carousel;-{
IMG_1881

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Tiled Gallery Customer Report Issues or PRs that were reported via Happiness. aka "Happiness Request", or "User Report" [Feature] Carousel A fullscreen modal appearing when clicking on an image in a gallery or tiled gallery. [Pri] Normal [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Development

No branches or pull requests