Rarst / fragment-cache

WordPress plugin for partial and async caching.
Other
140 stars 10 forks source link

Clashes with Jetpack galleries #3

Closed remkus closed 9 years ago

remkus commented 9 years ago

I just noticed that enabling fragment cache will render the Jetpack galleries (tiled and carrousel) useless. The Javascript throws an error resulting in not displaying the images or being able to close the modal.

Any idea why this would happen?

Rarst commented 9 years ago

Cannot reproduce as described.

Enabling Fragment Cache for me simply makes galleries go back to WP native. From quick look at code JetpackTiled_Gallery::gallery_already_redefined() prevents it from loading if shortcode is replaced, which Fragment Cache does for caching.

remkus commented 9 years ago

They do indeed revert to default, but have you clicked on one of the images inside a gallery?

Rarst commented 9 years ago

Yes, I get normal attachment page. No carousel, but no JS errors either.

Rarst commented 9 years ago

Which theme and Jetpack version are you using? I tried Twenty Twelve and cloned Jetpack master.

remkus commented 9 years ago

That's strange, but still.. the expected behaviour when you have Jetpack Tiled Galleries and / or the Jetpack Carrousel activated is that they work :)

I was seeing the behaviour on a Genesis Child Theme and Jetpack 3.0.2. Can provide a link outside of this issue if needed.

Rarst commented 9 years ago

Magic only goes so far. :) You don't expect PHP to run with static cached page, right?

This is not as much Fragment Cache breaking Jetpack, as Jetpack refusing to coexist with other gallery solutions (which is pretty reasonable), which Fragment Cache comes across as.

Galleries are fickle. Fragment Cache does reasonable effort and works peachy for native ones, but for any complicated third party gallery you would probably need to disable cache handler or roll your own.

Anything actionable you would like me to consider here? :)

remkus commented 9 years ago

I've pinged the Jetpack guys, but the only thing I can think of would be to do a check if the Jetpack modules are being used in class-fragment-cache.php

Rarst commented 9 years ago

Fragment Cache was designed more as platform than as specific limited solution. Caching any gallery under the sun isn't mission statement. Empowering developers to easily create fragment cache handlers, which might do anything like caching complicated galleries is. :)

If anyone has interest in creating custom handler for Jetpack functionality I'd be happy to help (I don't use Jetpack, so I am not interested in doing one myself).

georgestephanis commented 9 years ago

Yeah, since Jetpack's Tiled Galleries overload core's Gallery shortcode via filters, if that shortcode is changed to a different function, we wuss out and don't do anything, because we don't want to conflict with an unknown third-party provider.

https://github.com/Automattic/jetpack/blob/da8a21cd896d25f17055bda5d8326eaffa10760f/modules/tiled-gallery/tiled-gallery.php#L322-L330

Just added a filter here:

https://github.com/Automattic/jetpack/commit/ad556355fbeeceef2eb3e400d68dbf326e2421c2

So you can do add_filter( 'jetpack_tiled_gallery_shortcode_redefined', '__return_false' ); -- and that will make it run anyways.

I'm not sure about the carousel functionality and how that's being duckpunched.

Rarst commented 9 years ago

Ok, this will basically make tiles work with Fragment Cache peachy:

add_action( 'wp_enqueue_scripts', function () {
    add_filter( 'jetpack_tiled_gallery_shortcode_redefined', '__return_false' );
    Jetpack_Tiled_Gallery::default_scripts_and_styles();
} );

The carousel seems more involved cause it adds things via filters inside of gallery shortcode.

jeherve commented 9 years ago

I can't figure out how to install composer in my shared host environment, so I can't really test things, but you should be able to force Carousel to work with the jp_carousel_force_enable filter:

add_filter( 'jp_carousel_force_enable', '__return_true' );

Let me know if it helps. In the meantime, I'll keep trying to get composer to work :)

Rarst commented 9 years ago

I have shared Composer install instructions for SiteGround, also you can always install plugin locally and upload. :)

I had tried force enable carousel, but I wasn't getting scripts (remember - that is async cache, things are out of process) so gave up for today.

Will try to look more at it and maybe document in wiki.

Rarst commented 9 years ago

I documented snippet for tiled galleries in wiki. Took a stab at carousels, but just too much going on there for me to figure out without actively using them.

Wiki > Compatibility > Jetpack