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.

Sign up for GitHub

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

Jump to bottom

AMP: move checks for AMP requests later, inside modules. #11195

Merged
merged 1 commit into from
Jan 28, 2019
Merged

AMP: move checks for AMP requests later, inside modules. #11195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 1 addition & 23 deletions 3rd-party/class.jetpack-amp-support.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,9 @@ static function init() {
add_action( 'amp_post_template_footer', array( 'Jetpack_AMP_Support', 'add_stats_pixel' ) );
}

// carousel
add_filter( 'jp_carousel_maybe_disable', array( __CLASS__, 'is_amp_request' ) );

// sharing
add_filter( 'sharing_enqueue_scripts', array( __CLASS__, 'is_not_amp_request' ) );
add_filter( 'jetpack_sharing_counts', array( __CLASS__, 'is_not_amp_request' ) );
add_filter( 'sharing_js', array( __CLASS__, 'is_not_amp_request' ) );
// Sharing.
add_filter( 'jetpack_sharing_display_markup', array( 'Jetpack_AMP_Support', 'render_sharing_html' ), 10, 2 );

// disable lazy images
add_filter( 'lazyload_is_enabled', array( __CLASS__, 'is_not_amp_request' ) );

// disable imploding CSS
add_filter( 'jetpack_implode_frontend_css', array( __CLASS__, 'is_not_amp_request' ) );

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good that these are integrated with the modules they relate to.

// enforce freedom mode for videopress
add_filter( 'videopress_shortcode_options', array( 'Jetpack_AMP_Support', 'videopress_enable_freedom_mode' ) );

Expand Down Expand Up @@ -64,16 +52,6 @@ static function is_amp_request() {
return apply_filters( 'jetpack_is_amp_request', $is_amp_request );
}

/**
* Returns whether the request is not AMP.
*
* @see Jetpack_AMP_Support::is_amp_request()
* @return bool Whether not AMP.
*/
static function is_not_amp_request() {
return ! self::is_amp_request();
}

static function amp_disable_the_content_filters() {
if ( defined( 'WPCOM') && WPCOM ) {
add_filter( 'videopress_show_2015_player', '__return_true' );
Expand Down
5 changes: 5 additions & 0 deletions class.jetpack.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6873,6 +6873,11 @@ public function implode_frontend_css( $travis_test = false ) {
$do_implode = false;
}

// Do not implode CSS when the page loads via the AMP plugin.
if ( Jetpack_AMP_Support::is_amp_request() ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This call is made during wp_print_scripts so it will work well.

$do_implode = false;
}

/**
* Allow CSS to be concatenated into a single jetpack.css file.
*
Expand Down
6 changes: 3 additions & 3 deletions modules/carousel/jetpack-carousel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Jetpack_Carousel {
public $single_image_gallery_enabled_media_file = false;

function __construct() {
add_action( 'wp', array( $this, 'init' ), 99 );
add_action( 'init', array( $this, 'init' ) );
}

function init() {
Expand All @@ -44,7 +44,7 @@ function init() {

if ( is_admin() ) {
// Register the Carousel-related related settings
$this->register_settings();
add_action( 'admin_init', array( $this, 'register_settings' ), 5 );
if ( ! $this->in_jetpack ) {
if ( 0 == $this->test_1or0_option( get_option( 'carousel_enable_it' ), true ) ) {
return; // Carousel disabled, abort early, but still register setting so user can switch it back on
Expand Down Expand Up @@ -217,7 +217,7 @@ function check_content_for_blocks( $content ) {
if ( Jetpack_AMP_Support::is_amp_request() ) {
return $content;
}

if (
function_exists( 'has_block' )
&& ( has_block( 'gallery', $content ) || has_block( 'jetpack/tiled-gallery', $content ) )
Expand Down
12 changes: 4 additions & 8 deletions modules/lazy-images/lazy-images.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ private function __construct() {
return;
}

if ( Jetpack_AMP_Support::is_amp_request() ) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This is works because Jetpack_Lazy_Images is instantiated at the wp action:

jetpack/modules/lazy-images.php

Lines 29 to 40 in cb4dcbb

/*
* Initialize lazy images on the wp action so that conditional
* tags are safe to use.
*
* As an example, this is important if a theme wants to disable lazy images except
* on single posts, pages, or attachments by short-circuiting lazy images when
* is_singular() returns false.
*
* See: https://github.com/Automattic/jetpack/issues/8888
*/
add_action( 'wp', array( 'Jetpack_Lazy_Images', 'instance' ) );

It might be a good idea to mention AMP as being the reason for the wp action there.


add_action( 'wp_head', array( $this, 'setup_filters' ), 9999 ); // we don't really want to modify anything in <head> since it's mostly all metadata
add_action( 'wp_enqueue_scripts', array( $this, 'enqueue_assets' ) );

Expand Down Expand Up @@ -102,11 +106,6 @@ public function add_image_placeholders( $content ) {
return $content;
}

// Don't lazyload for amp-wp content
if ( Jetpack_AMP_Support::is_amp_request() ) {
return $content;
}

// This is a pretty simple regex, but it works
$content = preg_replace_callback( '#<(img)([^>]+?)(>(.*?)</\\1>|[\/]?>)#si', array( __CLASS__, 'process_image' ), $content );

Expand Down Expand Up @@ -340,9 +339,6 @@ private static function build_attributes_string( $attributes ) {
}

public function enqueue_assets() {
if ( Jetpack_AMP_Support::is_amp_request() ) {
return;
}
wp_enqueue_script(
'jetpack-lazy-images',
Jetpack::get_file_url_for_environment(
Expand Down
4 changes: 4 additions & 0 deletions modules/sharedaddy/sharing-service.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,10 @@ function sharing_maybe_enqueue_scripts() {
}

function sharing_add_footer() {
if ( Jetpack_AMP_Support::is_amp_request() ) {
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This will work fine because it will run at the wp_footer action, which happens after wp.


global $jetpack_sharing_counts;

/**
Expand Down