From edacdae17b231801cb09bfdb44606b29ac8f21d1 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 30 Oct 2020 21:49:37 -0700 Subject: [PATCH 001/169] Introduce context param for amp_add_paired_endpoint() --- includes/admin/class-amp-post-meta-box.php | 11 +- includes/amp-helper-functions.php | 163 ++++++++++++------ includes/class-amp-theme-support.php | 5 +- .../embeds/class-amp-core-block-handler.php | 2 +- .../sanitizers/class-amp-form-sanitizer.php | 2 +- .../sanitizers/class-amp-link-sanitizer.php | 2 +- includes/templates/amp-paired-browsing.php | 4 +- .../class-amp-validated-url-post-type.php | 2 +- .../class-amp-validation-manager.php | 5 +- src/MobileRedirection.php | 5 +- 10 files changed, 132 insertions(+), 69 deletions(-) diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index acae775f39f..17a51d0a9c6 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -110,7 +110,7 @@ public function init() { add_action( 'post_submitbox_misc_actions', [ $this, 'render_status' ] ); add_action( 'save_post', [ $this, 'save_amp_status' ] ); add_action( 'rest_api_init', [ $this, 'add_rest_api_fields' ] ); - add_filter( 'preview_post_link', [ $this, 'preview_post_link' ] ); + add_filter( 'preview_post_link', [ $this, 'preview_post_link' ], 10, 2 ); } /** @@ -188,7 +188,7 @@ public function enqueue_admin_assets() { 'ampPostMetaBox.boot( %s );', wp_json_encode( [ - 'previewLink' => esc_url_raw( amp_add_paired_endpoint( get_preview_post_link( $post ) ) ), + 'previewLink' => esc_url_raw( amp_add_paired_endpoint( get_preview_post_link( $post ), $post ) ), 'canonical' => amp_is_canonical(), 'enabled' => empty( $support_errors ), 'canSupport' => 0 === count( array_diff( $support_errors, [ 'post-status-disabled' ] ) ), @@ -436,10 +436,11 @@ public function save_amp_status( $post_id ) { * * @since 0.6 * - * @param string $link The post preview link. + * @param string $link The post preview link. + * @param WP_Post $post The post. * @return string Preview URL. */ - public function preview_post_link( $link ) { + public function preview_post_link( $link, $post ) { $is_amp = ( isset( $_POST['amp-preview'] ) // phpcs:ignore WordPress.Security.NonceVerification.Missing && @@ -447,7 +448,7 @@ public function preview_post_link( $link ) { ); if ( $is_amp ) { - $link = amp_add_paired_endpoint( $link ); + $link = amp_add_paired_endpoint( $link, $post ); } return $link; diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 97171bab9b3..440abbba127 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -609,14 +609,55 @@ function _amp_bootstrap_customizer() { * @return string URL to be redirected. */ function amp_redirect_old_slug_to_new_url( $link ) { + if ( ! amp_is_request() || amp_is_canonical() ) { + return $link; + } - if ( amp_is_request() && ! amp_is_canonical() ) { - if ( ! amp_is_legacy() ) { - $link = amp_add_paired_endpoint( $link ); + /** + * The following logic for defining the $id is copied from `wp_old_slug_redirect()`. + * This is needed because the post ID is not passed as a parameter to the `old_slug_redirect_url` filter. + * + * @see wp_old_slug_redirect() + * @var int|null + */ + $post_id = ( function () { + // Guess the current post type based on the query vars. + if ( get_query_var( 'post_type' ) ) { + $post_type = get_query_var( 'post_type' ); + } elseif ( get_query_var( 'attachment' ) ) { + $post_type = 'attachment'; + } elseif ( get_query_var( 'pagename' ) ) { + $post_type = 'page'; } else { - $link = trailingslashit( trailingslashit( $link ) . amp_get_slug() ); + $post_type = 'post'; } - } + + if ( is_array( $post_type ) ) { + if ( count( $post_type ) > 1 ) { + return null; + } + $post_type = reset( $post_type ); + } + + // Do not attempt redirect for hierarchical post types. + if ( is_post_type_hierarchical( $post_type ) ) { + return null; + } + + $id = _find_post_by_old_slug( $post_type ); + + if ( ! $id ) { + $id = _find_post_by_old_date( $post_type ); + } + + /** This filter is documented in wp-includes/query.php */ + $id = apply_filters( 'old_slug_redirect_post_id', $id ); + + return $id; + } )(); + + $post = $post_id ? get_post( $post_id ) : null; + $link = amp_add_paired_endpoint( $link, $post ); return $link; } @@ -699,36 +740,7 @@ function amp_get_current_url() { * @return string AMP permalink. */ function amp_get_permalink( $post_id ) { - /** - * Filters the AMP permalink to short-circuit normal generation. - * - * Returning a non-false value in this filter will cause the `get_permalink()` to get called and the `amp_get_permalink` filter to not apply. - * - * @since 0.4 - * @since 1.0 This filter does not apply when 'amp' theme support is present. - * - * @param false $url Short-circuited URL. - * @param int $post_id Post ID. - */ - $pre_url = apply_filters( 'amp_pre_get_permalink', false, $post_id ); - - if ( false !== $pre_url ) { - return $pre_url; - } - - $permalink = get_permalink( $post_id ); - $amp_url = amp_is_canonical() ? $permalink : amp_add_paired_endpoint( $permalink ); - - /** - * Filters AMP permalink. - * - * @since 0.2 - * @since 1.0 This filter does not apply when 'amp' theme support is present. - * - * @param false $amp_url AMP URL. - * @param int $post_id Post ID. - */ - return apply_filters( 'amp_get_permalink', $amp_url, $post_id ); + return amp_add_paired_endpoint( get_permalink( $post_id ), get_post( $post_id ) ); } /** @@ -751,8 +763,10 @@ function amp_remove_endpoint( $url ) { * If there are known validation errors for the current URL then do not output anything. * * @since 1.0 + * @global WP_Query $wp_the_query */ function amp_add_amphtml_link() { + global $wp_the_query; if ( amp_is_canonical() || @@ -792,12 +806,7 @@ function amp_add_amphtml_link() { return; } - if ( AMP_Theme_Support::is_paired_available() ) { - $amp_url = amp_add_paired_endpoint( amp_get_current_url() ); - } else { - $amp_url = amp_get_permalink( get_queried_object_id() ); - } - + $amp_url = amp_add_paired_endpoint( amp_get_current_url(), $wp_the_query ); if ( $amp_url ) { $amp_url = remove_query_arg( QueryVar::NOAMP, $amp_url ); printf( '', esc_url( $amp_url ) ); @@ -1852,23 +1861,20 @@ function amp_wp_kses_mustache( $markup ) { * @internal * * @param WP_Admin_Bar $wp_admin_bar Admin bar. + * @global WP_Query $wp_the_query */ function amp_add_admin_bar_view_link( $wp_admin_bar ) { + global $wp_the_query; if ( is_admin() || amp_is_canonical() || ! amp_is_available() ) { return; } $is_amp_request = amp_is_request(); - if ( $is_amp_request ) { - $href = amp_remove_paired_endpoint( amp_get_current_url() ); - } elseif ( is_singular() ) { - $href = amp_get_permalink( get_queried_object_id() ); // For sake of Reader mode. - } else { - $href = amp_add_paired_endpoint( amp_get_current_url() ); - } + $amp_url = amp_add_paired_endpoint( amp_get_current_url(), $wp_the_query ); + $non_amp_url = amp_remove_paired_endpoint( amp_get_current_url() ); - $href = remove_query_arg( QueryVar::NOAMP, $href ); + $amp_url = remove_query_arg( QueryVar::NOAMP, $amp_url ); $icon = $is_amp_request ? Icon::logo() : Icon::link(); $attr = [ @@ -1880,7 +1886,8 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) { [ 'id' => 'amp', 'title' => $icon->to_html( $attr ) . ' ' . esc_html__( 'AMP', 'amp' ), - 'href' => esc_url( $href ), + 'href' => esc_url( $is_amp_request ? $non_amp_url : $amp_url ), + // @todo This should have a tooltip to "View non-AMP version" if ! $is_amp_request. ] ); @@ -1889,7 +1896,7 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) { 'parent' => 'amp', 'id' => 'amp-view', 'title' => esc_html( $is_amp_request ? __( 'View non-AMP version', 'amp' ) : __( 'View AMP version', 'amp' ) ), - 'href' => esc_url( $href ), + 'href' => esc_url( $is_amp_request ? $non_amp_url : $amp_url ), ] ); @@ -1898,9 +1905,9 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) { if ( $customize_node && $is_amp_request && AMP_Theme_Support::READER_MODE_SLUG === AMP_Options_Manager::get_option( Option::THEME_SUPPORT ) ) { $args = get_object_vars( $customize_node ); if ( amp_is_legacy() ) { - $args['href'] = add_query_arg( 'autofocus[panel]', AMP_Template_Customizer::PANEL_ID, $args['href'] ); + $args['href'] = add_query_arg( 'autofocus[panel]', AMP_Template_Customizer::PANEL_ID, $amp_url ); } else { - $args['href'] = amp_add_paired_endpoint( $args['href'] ); + $args['href'] = add_query_arg( amp_get_slug(), '1', $args['href'] ); } $wp_admin_bar->add_node( $args ); } @@ -1940,11 +1947,55 @@ function amp_generate_script_hash( $script ) { * * @since 2.1 * - * @param string $url URL. + * @param string $url URL. + * @param WP_Query|WP_Post $context Context. * @return string AMP URL. */ -function amp_add_paired_endpoint( $url ) { - return add_query_arg( amp_get_slug(), '1', $url ); +function amp_add_paired_endpoint( $url, $context = null ) { + $post = null; + if ( $context instanceof WP_Query && $context->is_singular() ) { + $post = $context->get_queried_object(); + } elseif ( $context instanceof WP_Post ) { + $post = $context; + } + + if ( $post instanceof WP_Post ) { + /** + * Filters the AMP permalink to short-circuit normal generation. + * + * Returning a non-false value in this filter will cause the `get_permalink()` to get called and the `amp_get_permalink` filter to not apply. + * + * @since 0.4 + * @since 1.0 This filter does not apply when 'amp' theme support is present. + * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. + * + * @param false $url Short-circuited URL. + * @param int $post_id Post ID. + */ + $pre_url = apply_filters( 'amp_pre_get_permalink', false, $post->ID ); + + if ( false !== $pre_url ) { + return $pre_url; + } + } + + $amp_url = add_query_arg( amp_get_slug(), '1', $url ); + + if ( $post instanceof WP_Post ) { + /** + * Filters AMP permalink. + * + * @since 0.2 + * @since 1.0 This filter does not apply when 'amp' theme support is present. + * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. + * + * @param false $amp_url AMP URL. + * @param int $post_id Post ID. + */ + $amp_url = apply_filters( 'amp_get_permalink', $amp_url, $post->ID ); + } + + return $amp_url; } /** diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index eb23057231c..5920bf1d881 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -388,8 +388,11 @@ static function() { * @since 2.1 Remove obsolete redirection from /amp/ to ?amp when on non-legacy Reader mode. * * @return bool Whether redirection should have been done. + * + * @global WP_Query $wp_the_query */ public static function ensure_proper_amp_location() { + global $wp_the_query; if ( amp_is_canonical() ) { /* * When AMP-first/canonical, then when there is an /amp/ endpoint or ?amp URL param, @@ -413,7 +416,7 @@ public static function ensure_proper_amp_location() { wp_parse_str( $wp->matched_query, $path_args ); if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { $current_url = amp_get_current_url(); - $redirect_url = amp_add_paired_endpoint( amp_remove_paired_endpoint( $current_url ) ); + $redirect_url = amp_add_paired_endpoint( amp_remove_paired_endpoint( $current_url ), $wp_the_query ); if ( $current_url !== $redirect_url && wp_safe_redirect( $redirect_url, 301 ) ) { // @codeCoverageIgnoreStart exit; diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php index ce96e607327..41d486b578c 100644 --- a/includes/embeds/class-amp-core-block-handler.php +++ b/includes/embeds/class-amp-core-block-handler.php @@ -313,7 +313,7 @@ private function process_archives_widgets( Document $dom, $args = [] ) { * * @var DOMElement $option */ - $option->setAttribute( 'value', amp_add_paired_endpoint( $option->getAttribute( 'value' ) ) ); + $option->setAttribute( 'value', amp_add_paired_endpoint( $option->getAttribute( 'value' ), null ) ); } } } diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php index cd9885bed0b..37a06f713a2 100644 --- a/includes/sanitizers/class-amp-form-sanitizer.php +++ b/includes/sanitizers/class-amp-form-sanitizer.php @@ -87,7 +87,7 @@ public function sanitize() { // Record that action was converted to action-xhr. $action_url = add_query_arg( AMP_HTTP::ACTION_XHR_CONVERTED_QUERY_VAR, 1, $action_url ); if ( ! amp_is_canonical() ) { - $action_url = amp_add_paired_endpoint( $action_url ); + $action_url = amp_add_paired_endpoint( $action_url, null ); } $node->setAttribute( 'action-xhr', $action_url ); // Append success/error handlers if not found. diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index dc50477744c..cd96c23145e 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -234,7 +234,7 @@ private function process_element( DOMElement $element, $attribute_name ) { // Only add the AMP query var when requested (in Transitional or Reader mode). if ( ! $excluded && ! empty( $this->args['paired'] ) ) { - $url = amp_add_paired_endpoint( $url ); + $url = amp_add_paired_endpoint( $url, null ); } $element->setAttribute( $attribute_name, $url ); diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index 461fcd5b576..c5205a645d9 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -7,9 +7,11 @@ use AmpProject\AmpWP\QueryVar; +global $wp_the_query; + $url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ] ); $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, $url ); -$amp_url = amp_add_paired_endpoint( $url ); +$amp_url = amp_add_paired_endpoint( $url, $wp_the_query ); ?> diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 95d934d1b5a..13a6d47d1ab 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -726,7 +726,7 @@ public static function get_url_from_post( $post ) { // Add AMP query var if in transitional mode. if ( ! amp_is_canonical() ) { - $url = amp_add_paired_endpoint( $url ); + $url = amp_add_paired_endpoint( $url, $post ); } // Set URL scheme based on whether HTTPS is current. diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 6f0cde16897..72ea39b20b7 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -328,8 +328,11 @@ public static function is_sanitization_auto_accepted( $error = null ) { * @see amp_add_admin_bar_view_link() Where an admin bar item may have been added already for Reader/Transitional modes. * * @param WP_Admin_Bar $wp_admin_bar Admin bar. + * + * @global WP_Query $wp_the_query */ public static function add_admin_bar_menu_items( $wp_admin_bar ) { + global $wp_the_query; if ( is_admin() || ! self::get_dev_tools_user_access()->is_user_enabled() || ! amp_is_available() ) { self::$amp_admin_bar_item_added = false; return; @@ -350,7 +353,7 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) { $current_url ); if ( ! amp_is_canonical() ) { - $amp_url = amp_add_paired_endpoint( $amp_url ); + $amp_url = amp_add_paired_endpoint( $amp_url, $wp_the_query ); } $validate_url = AMP_Validated_URL_Post_Type::get_recheck_url( AMP_Validated_URL_Post_Type::get_invalid_url_post( $amp_url ) ?: $amp_url ); diff --git a/src/MobileRedirection.php b/src/MobileRedirection.php index d50626aaab0..03ffe77dd20 100644 --- a/src/MobileRedirection.php +++ b/src/MobileRedirection.php @@ -86,9 +86,12 @@ public function sanitize_options( $options, $new_options ) { * Get the AMP version of the current URL. * * @return string AMP URL. + * + * @global \WP_Query $wp_the_query */ public function get_current_amp_url() { - $url = amp_add_paired_endpoint( amp_get_current_url() ); + global $wp_the_query; + $url = amp_add_paired_endpoint( amp_get_current_url(), $wp_the_query ); $url = remove_query_arg( QueryVar::NOAMP, $url ); return $url; } From b6a9209135ca49f3b51420e7f0cf047f69015343 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 30 Oct 2020 23:03:47 -0700 Subject: [PATCH 002/169] Undo adding context param to amp_add_paired_endpoint() --- includes/admin/class-amp-post-meta-box.php | 11 ++- includes/amp-helper-functions.php | 85 +++++-------------- includes/class-amp-theme-support.php | 5 +- .../embeds/class-amp-core-block-handler.php | 2 +- .../sanitizers/class-amp-form-sanitizer.php | 2 +- .../sanitizers/class-amp-link-sanitizer.php | 2 +- includes/templates/amp-paired-browsing.php | 4 +- .../class-amp-validated-url-post-type.php | 2 +- .../class-amp-validation-manager.php | 5 +- src/MobileRedirection.php | 5 +- 10 files changed, 32 insertions(+), 91 deletions(-) diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index 17a51d0a9c6..acae775f39f 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -110,7 +110,7 @@ public function init() { add_action( 'post_submitbox_misc_actions', [ $this, 'render_status' ] ); add_action( 'save_post', [ $this, 'save_amp_status' ] ); add_action( 'rest_api_init', [ $this, 'add_rest_api_fields' ] ); - add_filter( 'preview_post_link', [ $this, 'preview_post_link' ], 10, 2 ); + add_filter( 'preview_post_link', [ $this, 'preview_post_link' ] ); } /** @@ -188,7 +188,7 @@ public function enqueue_admin_assets() { 'ampPostMetaBox.boot( %s );', wp_json_encode( [ - 'previewLink' => esc_url_raw( amp_add_paired_endpoint( get_preview_post_link( $post ), $post ) ), + 'previewLink' => esc_url_raw( amp_add_paired_endpoint( get_preview_post_link( $post ) ) ), 'canonical' => amp_is_canonical(), 'enabled' => empty( $support_errors ), 'canSupport' => 0 === count( array_diff( $support_errors, [ 'post-status-disabled' ] ) ), @@ -436,11 +436,10 @@ public function save_amp_status( $post_id ) { * * @since 0.6 * - * @param string $link The post preview link. - * @param WP_Post $post The post. + * @param string $link The post preview link. * @return string Preview URL. */ - public function preview_post_link( $link, $post ) { + public function preview_post_link( $link ) { $is_amp = ( isset( $_POST['amp-preview'] ) // phpcs:ignore WordPress.Security.NonceVerification.Missing && @@ -448,7 +447,7 @@ public function preview_post_link( $link, $post ) { ); if ( $is_amp ) { - $link = amp_add_paired_endpoint( $link, $post ); + $link = amp_add_paired_endpoint( $link ); } return $link; diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 440abbba127..2f7b5b3790c 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -609,55 +609,10 @@ function _amp_bootstrap_customizer() { * @return string URL to be redirected. */ function amp_redirect_old_slug_to_new_url( $link ) { - if ( ! amp_is_request() || amp_is_canonical() ) { - return $link; - } - - /** - * The following logic for defining the $id is copied from `wp_old_slug_redirect()`. - * This is needed because the post ID is not passed as a parameter to the `old_slug_redirect_url` filter. - * - * @see wp_old_slug_redirect() - * @var int|null - */ - $post_id = ( function () { - // Guess the current post type based on the query vars. - if ( get_query_var( 'post_type' ) ) { - $post_type = get_query_var( 'post_type' ); - } elseif ( get_query_var( 'attachment' ) ) { - $post_type = 'attachment'; - } elseif ( get_query_var( 'pagename' ) ) { - $post_type = 'page'; - } else { - $post_type = 'post'; - } - - if ( is_array( $post_type ) ) { - if ( count( $post_type ) > 1 ) { - return null; - } - $post_type = reset( $post_type ); - } - - // Do not attempt redirect for hierarchical post types. - if ( is_post_type_hierarchical( $post_type ) ) { - return null; - } - - $id = _find_post_by_old_slug( $post_type ); - - if ( ! $id ) { - $id = _find_post_by_old_date( $post_type ); - } - - /** This filter is documented in wp-includes/query.php */ - $id = apply_filters( 'old_slug_redirect_post_id', $id ); - - return $id; - } )(); - $post = $post_id ? get_post( $post_id ) : null; - $link = amp_add_paired_endpoint( $link, $post ); + if ( amp_is_request() && ! amp_is_canonical() ) { + $link = amp_add_paired_endpoint( $link ); + } return $link; } @@ -740,7 +695,7 @@ function amp_get_current_url() { * @return string AMP permalink. */ function amp_get_permalink( $post_id ) { - return amp_add_paired_endpoint( get_permalink( $post_id ), get_post( $post_id ) ); + return amp_add_paired_endpoint( get_permalink( $post_id ) ); } /** @@ -763,10 +718,8 @@ function amp_remove_endpoint( $url ) { * If there are known validation errors for the current URL then do not output anything. * * @since 1.0 - * @global WP_Query $wp_the_query */ function amp_add_amphtml_link() { - global $wp_the_query; if ( amp_is_canonical() || @@ -806,7 +759,7 @@ function amp_add_amphtml_link() { return; } - $amp_url = amp_add_paired_endpoint( amp_get_current_url(), $wp_the_query ); + $amp_url = amp_add_paired_endpoint( amp_get_current_url() ); if ( $amp_url ) { $amp_url = remove_query_arg( QueryVar::NOAMP, $amp_url ); printf( '', esc_url( $amp_url ) ); @@ -1861,20 +1814,17 @@ function amp_wp_kses_mustache( $markup ) { * @internal * * @param WP_Admin_Bar $wp_admin_bar Admin bar. - * @global WP_Query $wp_the_query */ function amp_add_admin_bar_view_link( $wp_admin_bar ) { - global $wp_the_query; if ( is_admin() || amp_is_canonical() || ! amp_is_available() ) { return; } $is_amp_request = amp_is_request(); - $amp_url = amp_add_paired_endpoint( amp_get_current_url(), $wp_the_query ); - $non_amp_url = amp_remove_paired_endpoint( amp_get_current_url() ); - - $amp_url = remove_query_arg( QueryVar::NOAMP, $amp_url ); + $current_url = remove_query_arg( QueryVar::NOAMP, amp_get_current_url() ); + $amp_url = amp_add_paired_endpoint( $current_url ); + $non_amp_url = amp_remove_paired_endpoint( $current_url ); $icon = $is_amp_request ? Icon::logo() : Icon::link(); $attr = [ @@ -1905,7 +1855,7 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) { if ( $customize_node && $is_amp_request && AMP_Theme_Support::READER_MODE_SLUG === AMP_Options_Manager::get_option( Option::THEME_SUPPORT ) ) { $args = get_object_vars( $customize_node ); if ( amp_is_legacy() ) { - $args['href'] = add_query_arg( 'autofocus[panel]', AMP_Template_Customizer::PANEL_ID, $amp_url ); + $args['href'] = add_query_arg( 'autofocus[panel]', AMP_Template_Customizer::PANEL_ID, $args['href'] ); } else { $args['href'] = add_query_arg( amp_get_slug(), '1', $args['href'] ); } @@ -1947,16 +1897,17 @@ function amp_generate_script_hash( $script ) { * * @since 2.1 * - * @param string $url URL. - * @param WP_Query|WP_Post $context Context. + * @param string $url URL. * @return string AMP URL. */ -function amp_add_paired_endpoint( $url, $context = null ) { +function amp_add_paired_endpoint( $url ) { + $post = null; - if ( $context instanceof WP_Query && $context->is_singular() ) { - $post = $context->get_queried_object(); - } elseif ( $context instanceof WP_Post ) { - $post = $context; + if ( has_filter( 'amp_pre_get_permalink' ) || has_filter( 'amp_get_permalink' ) ) { + $post_id = url_to_postid( $url ); + if ( $post_id ) { + $post = get_post( $post_id ); + } } if ( $post instanceof WP_Post ) { @@ -1968,6 +1919,7 @@ function amp_add_paired_endpoint( $url, $context = null ) { * @since 0.4 * @since 1.0 This filter does not apply when 'amp' theme support is present. * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. + * @todo Deprecate this filter? * * @param false $url Short-circuited URL. * @param int $post_id Post ID. @@ -1988,6 +1940,7 @@ function amp_add_paired_endpoint( $url, $context = null ) { * @since 0.2 * @since 1.0 This filter does not apply when 'amp' theme support is present. * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. + * @todo Deprecate this filter? * * @param false $amp_url AMP URL. * @param int $post_id Post ID. diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 5920bf1d881..eb23057231c 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -388,11 +388,8 @@ static function() { * @since 2.1 Remove obsolete redirection from /amp/ to ?amp when on non-legacy Reader mode. * * @return bool Whether redirection should have been done. - * - * @global WP_Query $wp_the_query */ public static function ensure_proper_amp_location() { - global $wp_the_query; if ( amp_is_canonical() ) { /* * When AMP-first/canonical, then when there is an /amp/ endpoint or ?amp URL param, @@ -416,7 +413,7 @@ public static function ensure_proper_amp_location() { wp_parse_str( $wp->matched_query, $path_args ); if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) { $current_url = amp_get_current_url(); - $redirect_url = amp_add_paired_endpoint( amp_remove_paired_endpoint( $current_url ), $wp_the_query ); + $redirect_url = amp_add_paired_endpoint( amp_remove_paired_endpoint( $current_url ) ); if ( $current_url !== $redirect_url && wp_safe_redirect( $redirect_url, 301 ) ) { // @codeCoverageIgnoreStart exit; diff --git a/includes/embeds/class-amp-core-block-handler.php b/includes/embeds/class-amp-core-block-handler.php index 41d486b578c..ce96e607327 100644 --- a/includes/embeds/class-amp-core-block-handler.php +++ b/includes/embeds/class-amp-core-block-handler.php @@ -313,7 +313,7 @@ private function process_archives_widgets( Document $dom, $args = [] ) { * * @var DOMElement $option */ - $option->setAttribute( 'value', amp_add_paired_endpoint( $option->getAttribute( 'value' ), null ) ); + $option->setAttribute( 'value', amp_add_paired_endpoint( $option->getAttribute( 'value' ) ) ); } } } diff --git a/includes/sanitizers/class-amp-form-sanitizer.php b/includes/sanitizers/class-amp-form-sanitizer.php index 37a06f713a2..cd9885bed0b 100644 --- a/includes/sanitizers/class-amp-form-sanitizer.php +++ b/includes/sanitizers/class-amp-form-sanitizer.php @@ -87,7 +87,7 @@ public function sanitize() { // Record that action was converted to action-xhr. $action_url = add_query_arg( AMP_HTTP::ACTION_XHR_CONVERTED_QUERY_VAR, 1, $action_url ); if ( ! amp_is_canonical() ) { - $action_url = amp_add_paired_endpoint( $action_url, null ); + $action_url = amp_add_paired_endpoint( $action_url ); } $node->setAttribute( 'action-xhr', $action_url ); // Append success/error handlers if not found. diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index cd96c23145e..dc50477744c 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -234,7 +234,7 @@ private function process_element( DOMElement $element, $attribute_name ) { // Only add the AMP query var when requested (in Transitional or Reader mode). if ( ! $excluded && ! empty( $this->args['paired'] ) ) { - $url = amp_add_paired_endpoint( $url, null ); + $url = amp_add_paired_endpoint( $url ); } $element->setAttribute( $attribute_name, $url ); diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index c5205a645d9..461fcd5b576 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -7,11 +7,9 @@ use AmpProject\AmpWP\QueryVar; -global $wp_the_query; - $url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ] ); $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, $url ); -$amp_url = amp_add_paired_endpoint( $url, $wp_the_query ); +$amp_url = amp_add_paired_endpoint( $url ); ?> diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 13a6d47d1ab..95d934d1b5a 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -726,7 +726,7 @@ public static function get_url_from_post( $post ) { // Add AMP query var if in transitional mode. if ( ! amp_is_canonical() ) { - $url = amp_add_paired_endpoint( $url, $post ); + $url = amp_add_paired_endpoint( $url ); } // Set URL scheme based on whether HTTPS is current. diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 72ea39b20b7..6f0cde16897 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -328,11 +328,8 @@ public static function is_sanitization_auto_accepted( $error = null ) { * @see amp_add_admin_bar_view_link() Where an admin bar item may have been added already for Reader/Transitional modes. * * @param WP_Admin_Bar $wp_admin_bar Admin bar. - * - * @global WP_Query $wp_the_query */ public static function add_admin_bar_menu_items( $wp_admin_bar ) { - global $wp_the_query; if ( is_admin() || ! self::get_dev_tools_user_access()->is_user_enabled() || ! amp_is_available() ) { self::$amp_admin_bar_item_added = false; return; @@ -353,7 +350,7 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) { $current_url ); if ( ! amp_is_canonical() ) { - $amp_url = amp_add_paired_endpoint( $amp_url, $wp_the_query ); + $amp_url = amp_add_paired_endpoint( $amp_url ); } $validate_url = AMP_Validated_URL_Post_Type::get_recheck_url( AMP_Validated_URL_Post_Type::get_invalid_url_post( $amp_url ) ?: $amp_url ); diff --git a/src/MobileRedirection.php b/src/MobileRedirection.php index 03ffe77dd20..d50626aaab0 100644 --- a/src/MobileRedirection.php +++ b/src/MobileRedirection.php @@ -86,12 +86,9 @@ public function sanitize_options( $options, $new_options ) { * Get the AMP version of the current URL. * * @return string AMP URL. - * - * @global \WP_Query $wp_the_query */ public function get_current_amp_url() { - global $wp_the_query; - $url = amp_add_paired_endpoint( amp_get_current_url(), $wp_the_query ); + $url = amp_add_paired_endpoint( amp_get_current_url() ); $url = remove_query_arg( QueryVar::NOAMP, $url ); return $url; } From 3420914f1bee4b47dbc808a399fbbc3af095ad94 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 1 Nov 2020 20:26:53 -0800 Subject: [PATCH 003/169] Introduce PairedAmpRouting service --- .phpstorm.meta.php | 1 + includes/amp-helper-functions.php | 120 +------ .../options/class-amp-options-manager.php | 5 +- src/AmpWpPlugin.php | 2 + src/Option.php | 9 + src/PairedAmpRouting.php | 318 ++++++++++++++++++ .../wordpress-defines.php | 1 + 7 files changed, 337 insertions(+), 119 deletions(-) create mode 100644 src/PairedAmpRouting.php diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php index e59900f24df..956cef6bc1f 100644 --- a/.phpstorm.meta.php +++ b/.phpstorm.meta.php @@ -33,6 +33,7 @@ 'server_timing' => \AmpProject\AmpWP\Instrumentation\ServerTiming::class, 'site_health_integration' => \AmpProject\AmpWP\Admin\SiteHealth::class, 'validated_url_stylesheet_gc' => \AmpProject\AmpWP\BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class, + 'paired_amp_routing' => \AmpProject\AmpWP\PairedAmpRouting::class, ] ) ); diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 2f7b5b3790c..f96cb251538 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -22,11 +22,6 @@ */ function amp_activate( $network_wide = false ) { AmpWpPluginFactory::create()->activate( $network_wide ); - amp_after_setup_theme(); - if ( ! did_action( 'amp_init' ) ) { - amp_init(); - } - flush_rewrite_rules(); } /** @@ -39,16 +34,6 @@ function amp_activate( $network_wide = false ) { */ function amp_deactivate( $network_wide = false ) { AmpWpPluginFactory::create()->deactivate( $network_wide ); - // We need to manually remove the amp endpoint. - global $wp_rewrite; - foreach ( $wp_rewrite->endpoints as $index => $endpoint ) { - if ( amp_get_slug() === $endpoint[1] ) { - unset( $wp_rewrite->endpoints[ $index ] ); - break; - } - } - - flush_rewrite_rules( false ); } /** @@ -122,7 +107,6 @@ function amp_init() { add_action( 'rest_api_init', 'AMP_Options_Manager::register_settings' ); add_action( 'wp_loaded', 'amp_bootstrap_admin' ); - add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK ); add_action( 'parse_query', 'amp_correct_query_when_is_front_page' ); add_action( 'admin_bar_menu', 'amp_add_admin_bar_view_link', 100 ); @@ -1901,54 +1885,7 @@ function amp_generate_script_hash( $script ) { * @return string AMP URL. */ function amp_add_paired_endpoint( $url ) { - - $post = null; - if ( has_filter( 'amp_pre_get_permalink' ) || has_filter( 'amp_get_permalink' ) ) { - $post_id = url_to_postid( $url ); - if ( $post_id ) { - $post = get_post( $post_id ); - } - } - - if ( $post instanceof WP_Post ) { - /** - * Filters the AMP permalink to short-circuit normal generation. - * - * Returning a non-false value in this filter will cause the `get_permalink()` to get called and the `amp_get_permalink` filter to not apply. - * - * @since 0.4 - * @since 1.0 This filter does not apply when 'amp' theme support is present. - * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. - * @todo Deprecate this filter? - * - * @param false $url Short-circuited URL. - * @param int $post_id Post ID. - */ - $pre_url = apply_filters( 'amp_pre_get_permalink', false, $post->ID ); - - if ( false !== $pre_url ) { - return $pre_url; - } - } - - $amp_url = add_query_arg( amp_get_slug(), '1', $url ); - - if ( $post instanceof WP_Post ) { - /** - * Filters AMP permalink. - * - * @since 0.2 - * @since 1.0 This filter does not apply when 'amp' theme support is present. - * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. - * @todo Deprecate this filter? - * - * @param false $amp_url AMP URL. - * @param int $post_id Post ID. - */ - $amp_url = apply_filters( 'amp_get_permalink', $amp_url, $post->ID ); - } - - return $amp_url; + return Services::get( 'paired_amp_routing' )->add_paired_endpoint( $url ); } /** @@ -1958,45 +1895,9 @@ function amp_add_paired_endpoint( $url ) { * * @param string $url URL to examine. If empty, will use the current URL. * @return bool True if the AMP query parameter is set with the required value, false if not. - * @global WP_Query $wp_query */ function amp_has_paired_endpoint( $url = '' ) { - $slug = amp_get_slug(); - - // If the URL was not provided, then use the environment which is already parsed. - if ( empty( $url ) ) { - global $wp_query; - return ( - isset( $_GET[ $slug ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended - || - ( - $wp_query instanceof WP_Query - && - false !== $wp_query->get( $slug, false ) - ) - ); - } - - $parsed_url = wp_parse_url( $url ); - if ( ! empty( $parsed_url['query'] ) ) { - $query_vars = []; - wp_parse_str( $parsed_url['query'], $query_vars ); - if ( isset( $query_vars[ $slug ] ) ) { - return true; - } - } - - if ( ! empty( $parsed_url['path'] ) ) { - $pattern = sprintf( - '#/%s(/[^/^])?/?$#', - preg_quote( $slug, '#' ) - ); - if ( preg_match( $pattern, $parsed_url['path'] ) ) { - return true; - } - } - - return false; + return Services::get( 'paired_amp_routing' )->has_paired_endpoint( $url ); } /** @@ -2008,20 +1909,5 @@ function amp_has_paired_endpoint( $url = '' ) { * @return string URL with AMP stripped. */ function amp_remove_paired_endpoint( $url ) { - $slug = amp_get_slug(); - - // Strip endpoint, including /amp/, /amp/amp/, /amp/foo/. - $url = preg_replace( - sprintf( - ':(/%s(/[^/?#]+)?)+(?=/?(\?|#|$)):', - preg_quote( $slug, ':' ) - ), - '', - $url - ); - - // Strip query var, including ?amp, ?amp=1, etc. - $url = remove_query_arg( $slug, $url ); - - return $url; + return Services::get( 'paired_amp_routing' )->remove_paired_endpoint( $url ); } diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php index d590b9a6337..af9fd08720a 100644 --- a/includes/options/class-amp-options-manager.php +++ b/includes/options/class-amp-options-manager.php @@ -159,9 +159,10 @@ static function ( $supported ) { * Filters default options. * * @internal - * @param array $defaults Default options. + * @param array $defaults Default options. + * @param array $current_options Current options. */ - (array) apply_filters( 'amp_default_options', $defaults ), + (array) apply_filters( 'amp_default_options', $defaults, $options ), $options ); diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php index 18ae82e38d1..6275b6e70a6 100644 --- a/src/AmpWpPlugin.php +++ b/src/AmpWpPlugin.php @@ -80,6 +80,7 @@ final class AmpWpPlugin extends ServiceBasedPlugin { 'server_timing' => Instrumentation\ServerTiming::class, 'site_health_integration' => Admin\SiteHealth::class, 'validated_url_stylesheet_gc' => BackgroundTask\ValidatedUrlStylesheetDataGarbageCollection::class, + 'paired_amp_routing' => PairedAmpRouting::class, ]; /** @@ -161,6 +162,7 @@ protected function get_shared_instances() { DevTools\CallbackReflection::class, DevTools\FileReflection::class, ReaderThemeLoader::class, + PairedAmpRouting::class, ]; } diff --git a/src/Option.php b/src/Option.php index 3416f38cb21..39c622b6645 100644 --- a/src/Option.php +++ b/src/Option.php @@ -43,6 +43,15 @@ interface Option { */ const DISABLE_CSS_TRANSIENT_CACHING = 'amp_css_transient_monitor_disable_caching'; + /** + * Indicate the structure for Paired AMP URLs. + * + * Default value: 'query_var' + * + * @var string + */ + const PERMALINK_STRUCTURE = 'permalink_structure'; + /** * Redirect mobile visitors to the AMP version of a page when the site is in Transitional or Reader mode. * diff --git a/src/PairedAmpRouting.php b/src/PairedAmpRouting.php new file mode 100644 index 00000000000..5c1f03812c8 --- /dev/null +++ b/src/PairedAmpRouting.php @@ -0,0 +1,318 @@ +flush_rewrite_rules(); + } else { + add_action( 'init', [ $this, 'flush_rewrite_rules' ], 0 ); + } + } + + /** + * Deactivate. + * + * @param bool $network_wide Network-wide. + */ + public function deactivate( $network_wide ) { + unset( $network_wide ); + + // We need to manually remove the amp endpoint. + global $wp_rewrite; + foreach ( $wp_rewrite->endpoints as $index => $endpoint ) { + if ( amp_get_slug() === $endpoint[1] ) { + unset( $wp_rewrite->endpoints[ $index ] ); + break; + } + } + + $this->flush_rewrite_rules(); + } + + /** + * Register. + */ + public function register() { + add_filter( 'amp_default_options', [ $this, 'filter_default_options' ], 10, 2 ); + add_filter( 'amp_options_updating', [ $this, 'sanitize_options' ], 10, 2 ); + + add_action( 'init', [ $this, 'add_rewrite_endpoint' ], 0 ); + } + + /** + * Flush rewrite rules. + */ + public function flush_rewrite_rules() { + flush_rewrite_rules( false ); + } + + /** + * Add rewrite endpoint. + */ + public function add_rewrite_endpoint() { + if ( self::PERMALINK_STRUCTURE_ENDPOINT === AMP_Options_Manager::get_option( Option::PERMALINK_STRUCTURE ) ) { + $places = EP_ALL; + } else { + $places = EP_PERMALINK; + } + add_rewrite_endpoint( amp_get_slug(), $places ); + } + + /** + * Add default option. + * + * @param array $defaults Default options. + * @param array $options Current options. + * @return array Defaults. + */ + public function filter_default_options( $defaults, $options ) { + $value = self::PERMALINK_STRUCTURE_QUERY_VAR; + + if ( + isset( $options[ Option::VERSION ], $options[ Option::THEME_SUPPORT ], $options[ Option::READER_THEME ] ) + && + version_compare( $options[ Option::VERSION ], '2.1', '<' ) + ) { + if ( + ReaderThemes::DEFAULT_READER_THEME === $options[ Option::READER_THEME ] + && + AMP_Theme_Support::READER_MODE_SLUG === $options[ Option::THEME_SUPPORT ] + ) { + $value = self::PERMALINK_STRUCTURE_LEGACY_READER; + } elseif ( AMP_Theme_Support::STANDARD_MODE_SLUG !== $options[ Option::THEME_SUPPORT ] ) { + $value = self::PERMALINK_STRUCTURE_LEGACY_TRANSITIONAL; + } + } + + $defaults[ Option::PERMALINK_STRUCTURE ] = $value; + + return $defaults; + } + + /** + * Sanitize options. + * + * @param array $options Existing options with already-sanitized values for updating. + * @param array $new_options Unsanitized options being submitted for updating. + * + * @return array Sanitized options. + */ + public function sanitize_options( $options, $new_options ) { + if ( + isset( $new_options[ Option::PERMALINK_STRUCTURE ] ) + && + in_array( $new_options[ Option::PERMALINK_STRUCTURE ], self::PERMALINK_STRUCTURES, true ) + ) { + $options[ Option::PERMALINK_STRUCTURE ] = $new_options[ Option::PERMALINK_STRUCTURE ]; + } + return $options; + } + + /** + * Turn a given URL into a paired AMP URL. + * + * @param string $url URL. + * @return string AMP URL. + */ + public function add_paired_endpoint( $url ) { + $post = null; + if ( has_filter( 'amp_pre_get_permalink' ) || has_filter( 'amp_get_permalink' ) ) { + $post_id = url_to_postid( $url ); + if ( $post_id ) { + $post = get_post( $post_id ); + } + } + + if ( $post instanceof WP_Post ) { + /** + * Filters the AMP permalink to short-circuit normal generation. + * + * Returning a non-false value in this filter will cause the `get_permalink()` to get called and the `amp_get_permalink` filter to not apply. + * + * @since 0.4 + * @since 1.0 This filter does not apply when 'amp' theme support is present. + * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. + * @todo Deprecate this filter? + * + * @param false $url Short-circuited URL. + * @param int $post_id Post ID. + */ + $pre_url = apply_filters( 'amp_pre_get_permalink', false, $post->ID ); + + if ( false !== $pre_url ) { + return $pre_url; + } + } + + $amp_url = add_query_arg( amp_get_slug(), '1', $url ); + + if ( $post instanceof WP_Post ) { + /** + * Filters AMP permalink. + * + * @since 0.2 + * @since 1.0 This filter does not apply when 'amp' theme support is present. + * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. + * @todo Deprecate this filter? + * + * @param false $amp_url AMP URL. + * @param int $post_id Post ID. + */ + $amp_url = apply_filters( 'amp_get_permalink', $amp_url, $post->ID ); + } + + return $amp_url; + } + + /** + * Determine a given URL is for a paired AMP request. + * + * @param string $url URL to examine. If empty, will use the current URL. + * @return bool True if the AMP query parameter is set with the required value, false if not. + * @global WP_Query $wp_query + */ + public function has_paired_endpoint( $url = '' ) { + $slug = amp_get_slug(); + + // If the URL was not provided, then use the environment which is already parsed. + if ( empty( $url ) ) { + global $wp_query; + return ( + isset( $_GET[ $slug ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + || + ( + $wp_query instanceof WP_Query + && + false !== $wp_query->get( $slug, false ) + ) + ); + } + + $parsed_url = wp_parse_url( $url ); + if ( ! empty( $parsed_url['query'] ) ) { + $query_vars = []; + wp_parse_str( $parsed_url['query'], $query_vars ); + if ( isset( $query_vars[ $slug ] ) ) { + return true; + } + } + + if ( ! empty( $parsed_url['path'] ) ) { + $pattern = sprintf( + '#/%s(/[^/^])?/?$#', + preg_quote( $slug, '#' ) + ); + if ( preg_match( $pattern, $parsed_url['path'] ) ) { + return true; + } + } + + return false; + } + + /** + * Remove the paired AMP endpoint from a given URL. + * + * @param string $url URL. + * @return string URL with AMP stripped. + */ + public function remove_paired_endpoint( $url ) { + $slug = amp_get_slug(); + + // Strip endpoint, including /amp/, /amp/amp/, /amp/foo/. + $url = preg_replace( + sprintf( + ':(/%s(/[^/?#]+)?)+(?=/?(\?|#|$)):', + preg_quote( $slug, ':' ) + ), + '', + $url + ); + + // Strip query var, including ?amp, ?amp=1, etc. + $url = remove_query_arg( $slug, $url ); + + return $url; + } +} diff --git a/tests/php/static-analysis-stubs/wordpress-defines.php b/tests/php/static-analysis-stubs/wordpress-defines.php index 1b1180bcece..ee98688c490 100644 --- a/tests/php/static-analysis-stubs/wordpress-defines.php +++ b/tests/php/static-analysis-stubs/wordpress-defines.php @@ -15,6 +15,7 @@ define( 'WPINC', 'some_string' ); define( 'EMPTY_TRASH_DAYS', 30 * 86400 ); define( 'EP_PERMALINK', 1 ); +define( 'EP_ALL', 8191 ); define( 'COOKIE_DOMAIN', false ); // Constants for expressing human-readable intervals. From 162b13a31e7002946844fd765311ccd3dac740ca Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 1 Nov 2020 20:38:13 -0800 Subject: [PATCH 004/169] Move amp_redirect_old_slug_to_new_url() to PairedAmpRouting --- includes/amp-helper-functions.php | 23 ----------------------- includes/deprecated.php | 19 +++++++++++++++++++ src/PairedAmpRouting.php | 19 +++++++++++++++++++ 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index f96cb251538..52b1dbada63 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -124,9 +124,6 @@ function () { add_action( 'wp_loaded', 'amp_editor_core_blocks' ); add_filter( 'request', 'amp_force_query_var_value' ); - // Redirect the old url of amp page to the updated url. - add_filter( 'old_slug_redirect_url', 'amp_redirect_old_slug_to_new_url' ); - if ( defined( 'WP_CLI' ) && WP_CLI ) { if ( class_exists( 'WP_CLI\Dispatcher\CommandNamespace' ) ) { WP_CLI::add_command( 'amp', 'AMP_CLI_Namespace' ); @@ -581,26 +578,6 @@ function _amp_bootstrap_customizer() { add_action( 'after_setup_theme', 'amp_init_customizer', 12 ); } -/** - * Redirects the old AMP URL to the new AMP URL. - * - * If post slug is updated the amp page with old post slug will be redirected to the updated url. - * - * @since 0.5 - * @internal - * - * @param string $link New URL of the post. - * @return string URL to be redirected. - */ -function amp_redirect_old_slug_to_new_url( $link ) { - - if ( amp_is_request() && ! amp_is_canonical() ) { - $link = amp_add_paired_endpoint( $link ); - } - - return $link; -} - /** * Get the slug used in AMP for the query var, endpoint, and post type support. * diff --git a/includes/deprecated.php b/includes/deprecated.php index 7472bca5b55..65dcf437bd7 100644 --- a/includes/deprecated.php +++ b/includes/deprecated.php @@ -5,6 +5,8 @@ * @package AMP */ +use AmpProject\AmpWP\Services; + /** * Load classes. * @@ -282,3 +284,20 @@ function _amp_xdebug_admin_notice() { filter_old_slug_redirect_url( $link ); +} diff --git a/src/PairedAmpRouting.php b/src/PairedAmpRouting.php index 5c1f03812c8..d52cabde65f 100644 --- a/src/PairedAmpRouting.php +++ b/src/PairedAmpRouting.php @@ -118,6 +118,8 @@ public function register() { add_filter( 'amp_options_updating', [ $this, 'sanitize_options' ], 10, 2 ); add_action( 'init', [ $this, 'add_rewrite_endpoint' ], 0 ); + + add_filter( 'old_slug_redirect_url', [ $this, 'filter_old_slug_redirect_url' ] ); } /** @@ -315,4 +317,21 @@ public function remove_paired_endpoint( $url ) { return $url; } + + /** + * Redirects the old AMP URL to the new AMP URL. + * + * If post slug is updated the amp page with old post slug will be redirected to the updated url. + * + * @param string $url New URL of the post. + * @return string URL to be redirected. + */ + public function filter_old_slug_redirect_url( $url ) { + + if ( amp_is_request() && ! amp_is_canonical() ) { + $url = amp_add_paired_endpoint( $url ); + } + + return $url; + } } From 33116c6c48236008655dfb2d7ca5523f3b69de39 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 1 Nov 2020 20:44:03 -0800 Subject: [PATCH 005/169] Move amp_correct_query_when_is_front_page() to PairedAmpRouting service --- includes/amp-helper-functions.php | 42 ------------------------------- includes/deprecated.php | 18 +++++++++++++ src/PairedAmpRouting.php | 40 +++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 42 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 52b1dbada63..cabde0e6f7f 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -107,7 +107,6 @@ function amp_init() { add_action( 'rest_api_init', 'AMP_Options_Manager::register_settings' ); add_action( 'wp_loaded', 'amp_bootstrap_admin' ); - add_action( 'parse_query', 'amp_correct_query_when_is_front_page' ); add_action( 'admin_bar_menu', 'amp_add_admin_bar_view_link', 100 ); add_action( @@ -259,47 +258,6 @@ function amp_force_query_var_value( $query_vars ) { return $query_vars; } -/** - * Fix up WP_Query for front page when amp query var is present. - * - * Normally the front page would not get served if a query var is present other than preview, page, paged, and cpage. - * - * @since 0.6 - * @internal - * @see WP_Query::parse_query() - * @link https://github.com/WordPress/wordpress-develop/blob/0baa8ae85c670d338e78e408f8d6e301c6410c86/src/wp-includes/class-wp-query.php#L951-L971 - * - * @param WP_Query $query Query. - */ -function amp_correct_query_when_is_front_page( WP_Query $query ) { - $is_front_page_query = ( - $query->is_main_query() - && - $query->is_home() - && - // Is AMP endpoint. - false !== $query->get( amp_get_slug(), false ) - && - // Is query not yet fixed uo up to be front page. - ! $query->is_front_page() - && - // Is showing pages on front. - 'page' === get_option( 'show_on_front' ) - && - // Has page on front set. - get_option( 'page_on_front' ) - && - // See line in WP_Query::parse_query() at . - 0 === count( array_diff( array_keys( wp_parse_args( $query->query ) ), [ amp_get_slug(), 'preview', 'page', 'paged', 'cpage' ] ) ) - ); - if ( $is_front_page_query ) { - $query->is_home = false; - $query->is_page = true; - $query->is_singular = true; - $query->set( 'page_id', get_option( 'page_on_front' ) ); - } -} - /** * Whether this is in 'canonical mode'. * diff --git a/includes/deprecated.php b/includes/deprecated.php index 65dcf437bd7..3a4f615ecd1 100644 --- a/includes/deprecated.php +++ b/includes/deprecated.php @@ -301,3 +301,21 @@ function amp_redirect_old_slug_to_new_url( $link ) { _deprecated_function( __FUNCTION__, '2.1' ); return Services::get( 'paired_amp_routing' )->filter_old_slug_redirect_url( $link ); } + +/** + * Fix up WP_Query for front page when amp query var is present. + * + * Normally the front page would not get served if a query var is present other than preview, page, paged, and cpage. + * + * @since 0.6 + * @internal + * @see WP_Query::parse_query() + * @link https://github.com/WordPress/wordpress-develop/blob/0baa8ae85c670d338e78e408f8d6e301c6410c86/src/wp-includes/class-wp-query.php#L951-L971 + * @deprecated + * + * @param WP_Query $query Query. + */ +function amp_correct_query_when_is_front_page( WP_Query $query ) { + _deprecated_function( __FUNCTION__, '2.1' ); + Services::get( 'paired_amp_routing' )->correct_query_when_is_front_page( $query ); +} diff --git a/src/PairedAmpRouting.php b/src/PairedAmpRouting.php index d52cabde65f..836b83ae6d0 100644 --- a/src/PairedAmpRouting.php +++ b/src/PairedAmpRouting.php @@ -118,6 +118,7 @@ public function register() { add_filter( 'amp_options_updating', [ $this, 'sanitize_options' ], 10, 2 ); add_action( 'init', [ $this, 'add_rewrite_endpoint' ], 0 ); + add_action( 'parse_query', [ $this, 'correct_query_when_is_front_page' ] ); add_filter( 'old_slug_redirect_url', [ $this, 'filter_old_slug_redirect_url' ] ); } @@ -318,6 +319,45 @@ public function remove_paired_endpoint( $url ) { return $url; } + /** + * Fix up WP_Query for front page when amp query var is present. + * + * Normally the front page would not get served if a query var is present other than preview, page, paged, and cpage. + * + * @see WP_Query::parse_query() + * @link https://github.com/WordPress/wordpress-develop/blob/0baa8ae85c670d338e78e408f8d6e301c6410c86/src/wp-includes/class-wp-query.php#L951-L971 + * + * @param WP_Query $query Query. + */ + public function correct_query_when_is_front_page( WP_Query $query ) { + $is_front_page_query = ( + $query->is_main_query() + && + $query->is_home() + && + // Is AMP endpoint. + false !== $query->get( amp_get_slug(), false ) + && + // Is query not yet fixed uo up to be front page. + ! $query->is_front_page() + && + // Is showing pages on front. + 'page' === get_option( 'show_on_front' ) + && + // Has page on front set. + get_option( 'page_on_front' ) + && + // See line in WP_Query::parse_query() at . + 0 === count( array_diff( array_keys( wp_parse_args( $query->query ) ), [ amp_get_slug(), 'preview', 'page', 'paged', 'cpage' ] ) ) + ); + if ( $is_front_page_query ) { + $query->is_home = false; + $query->is_page = true; + $query->is_singular = true; + $query->set( 'page_id', get_option( 'page_on_front' ) ); + } + } + /** * Redirects the old AMP URL to the new AMP URL. * From 0278c8c3f4fd3dd043fb149c24e38070e5fe44e0 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Nov 2020 09:26:13 -0800 Subject: [PATCH 006/169] Add initial implementations of the 4 paired AMP permalink structures --- src/PairedAmpRouting.php | 163 ++++++++++++++++++++++++++++++++------- 1 file changed, 137 insertions(+), 26 deletions(-) diff --git a/src/PairedAmpRouting.php b/src/PairedAmpRouting.php index 836b83ae6d0..16c2b07560a 100644 --- a/src/PairedAmpRouting.php +++ b/src/PairedAmpRouting.php @@ -20,6 +20,8 @@ /** * Service for routing users to and from paired AMP URLs. * + * @todo Add 404 redirection to non-AMP version. + * * @package AmpProject\AmpWP * @since 2.1 * @internal @@ -36,14 +38,14 @@ final class PairedAmpRouting implements Service, Registerable, Activateable, Dea const PERMALINK_STRUCTURE_QUERY_VAR = 'query_var'; /** - * Endpoint permalink structure. + * Rewrite endpoint permalink structure. * * This adds `/amp/` to all URLs, even pages and archives. This is a popular option for those who feel query params * are bad for SEO. * * @var string */ - const PERMALINK_STRUCTURE_ENDPOINT = 'endpoint'; + const PERMALINK_STRUCTURE_REWRITE_ENDPOINT = 'rewrite_endpoint'; /** * Legacy transitional permalink structure. @@ -71,7 +73,7 @@ final class PairedAmpRouting implements Service, Registerable, Activateable, Dea */ const PERMALINK_STRUCTURES = [ self::PERMALINK_STRUCTURE_QUERY_VAR, - self::PERMALINK_STRUCTURE_ENDPOINT, + self::PERMALINK_STRUCTURE_REWRITE_ENDPOINT, self::PERMALINK_STRUCTURE_LEGACY_TRANSITIONAL, self::PERMALINK_STRUCTURE_LEGACY_READER, ]; @@ -134,7 +136,7 @@ public function flush_rewrite_rules() { * Add rewrite endpoint. */ public function add_rewrite_endpoint() { - if ( self::PERMALINK_STRUCTURE_ENDPOINT === AMP_Options_Manager::get_option( Option::PERMALINK_STRUCTURE ) ) { + if ( self::PERMALINK_STRUCTURE_REWRITE_ENDPOINT === AMP_Options_Manager::get_option( Option::PERMALINK_STRUCTURE ) ) { $places = EP_ALL; } else { $places = EP_PERMALINK; @@ -158,9 +160,9 @@ public function filter_default_options( $defaults, $options ) { version_compare( $options[ Option::VERSION ], '2.1', '<' ) ) { if ( - ReaderThemes::DEFAULT_READER_THEME === $options[ Option::READER_THEME ] - && AMP_Theme_Support::READER_MODE_SLUG === $options[ Option::THEME_SUPPORT ] + && + ReaderThemes::DEFAULT_READER_THEME === $options[ Option::READER_THEME ] ) { $value = self::PERMALINK_STRUCTURE_LEGACY_READER; } elseif ( AMP_Theme_Support::STANDARD_MODE_SLUG !== $options[ Option::THEME_SUPPORT ] ) { @@ -195,54 +197,163 @@ public function sanitize_options( $options, $new_options ) { /** * Turn a given URL into a paired AMP URL. * + * @todo Check if has_filter( 'amp_add_paired_endpoint' ) to defer to custom endpoint. + * * @param string $url URL. * @return string AMP URL. */ public function add_paired_endpoint( $url ) { - $post = null; - if ( has_filter( 'amp_pre_get_permalink' ) || has_filter( 'amp_get_permalink' ) ) { - $post_id = url_to_postid( $url ); - if ( $post_id ) { - $post = get_post( $post_id ); + + switch ( AMP_Options_Manager::get_option( Option::PERMALINK_STRUCTURE ) ) { + case self::PERMALINK_STRUCTURE_REWRITE_ENDPOINT: + return $this->get_rewrite_endpoint_paired_amp_url( $url ); + case self::PERMALINK_STRUCTURE_LEGACY_TRANSITIONAL: + return $this->get_legacy_transitional_paired_amp_url( $url ); + case self::PERMALINK_STRUCTURE_LEGACY_READER: + return $this->get_legacy_reader_paired_amp_url( $url ); + } + + // This is the PERMALINK_STRUCTURE_QUERY_VAR case, the default. + return $this->get_query_var_paired_amp_url( $url ); + } + + /** + * Get paired AMP URL using query var (`?amp=1`). + * + * @param string $url URL. + * @return string AMP URL. + */ + public function get_query_var_paired_amp_url( $url ) { + return add_query_arg( amp_get_slug(), '1', $url ); + } + + /** + * Get paired AMP URL using a rewrite endpoint. + * + * @todo What are the scenarios where this conflicts with other rewrite endpoints also added? + * + * @param string $url URL. + * @return string AMP URL. + */ + public function get_rewrite_endpoint_paired_amp_url( $url ) { + $url = $this->remove_paired_endpoint( $url ); + + $parsed_url = array_merge( + wp_parse_url( home_url( '/' ) ), + wp_parse_url( $url ) + ); + if ( empty( $parsed_url['scheme'] ) ) { + $parsed_url['scheme'] = is_ssl() ? 'https' : 'http'; + } + if ( ! isset( $parsed_url['host'] ) ) { + $parsed_url['host'] = isset( $_SERVER['HTTP_HOST'] ) ? wp_unslash( $_SERVER['HTTP_HOST'] ) : 'localhost'; + } + + $parsed_url['path'] = trailingslashit( $parsed_url['path'] ); + $parsed_url['path'] .= user_trailingslashit( amp_get_slug(), 'amp' ); + + $amp_url = $parsed_url['scheme'] . '://'; + if ( isset( $parsed_url['user'] ) ) { + $amp_url .= $parsed_url['user']; + if ( isset( $parsed_url['pass'] ) ) { + $amp_url .= ':' . $parsed_url['pass']; } + $amp_url .= '@'; + } + $amp_url .= $parsed_url['host']; + if ( isset( $parsed_url['port'] ) ) { + $amp_url .= ':' . $parsed_url['port']; + } + $amp_url .= $parsed_url['path']; + if ( isset( $parsed_url['query'] ) ) { + $amp_url .= '?' . $parsed_url['query']; + } + if ( isset( $parsed_url['fragment'] ) ) { + $amp_url .= '#' . $parsed_url['fragment']; } + return $amp_url; + } + + /** + * Get paired AMP URL using the legacy transitional scheme (`?amp`). + * + * @param string $url URL. + * @return string AMP URL. + */ + public function get_legacy_transitional_paired_amp_url( $url ) { + return add_query_arg( amp_get_slug(), '', $url ); + } - if ( $post instanceof WP_Post ) { + /** + * Get paired AMP URL using the legacy reader scheme (`/amp/` or else `?amp`). + * + * @param string $url URL. + * @return string AMP URL. + */ + public function get_legacy_reader_paired_amp_url( $url ) { + $post_id = url_to_postid( $url ); + + if ( $post_id ) { /** * Filters the AMP permalink to short-circuit normal generation. * - * Returning a non-false value in this filter will cause the `get_permalink()` to get called and the `amp_get_permalink` filter to not apply. + * Returning a string value in this filter will bypass the `get_permalink()` from being called and the `amp_get_permalink` filter will not apply. * * @since 0.4 - * @since 1.0 This filter does not apply when 'amp' theme support is present. - * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. - * @todo Deprecate this filter? + * @since 1.0 This filter only applies when using the legacy reader permalink structure. * * @param false $url Short-circuited URL. * @param int $post_id Post ID. */ - $pre_url = apply_filters( 'amp_pre_get_permalink', false, $post->ID ); + $pre_url = apply_filters( 'amp_pre_get_permalink', false, $post_id ); - if ( false !== $pre_url ) { + if ( is_string( $pre_url ) ) { return $pre_url; } } - $amp_url = add_query_arg( amp_get_slug(), '1', $url ); + // Make sure any existing AMP endpoint is removed. + $url = $this->remove_paired_endpoint( $url ); + + $parsed_url = wp_parse_url( $url ); + $structure = get_option( 'permalink_structure' ); + $use_query_var = ( + // If pretty permalinks aren't available, then query var must be used. + empty( $structure ) + || + // If there are existing query vars, then always use the amp query var as well. + ! empty( $parsed_url['query'] ) + || + // If no post was found for the URL. + ! $post_id + || + // If the post type is hierarchical then the /amp/ endpoint isn't available. + is_post_type_hierarchical( get_post_type( $post_id ) ) + || + // Attachment pages don't accept the /amp/ endpoint. + 'attachment' === get_post_type( $post_id ) + ); + if ( $use_query_var ) { + $amp_url = add_query_arg( amp_get_slug(), '', $url ); + } else { + $amp_url = preg_replace( '/#.*/', '', $url ); + $amp_url = trailingslashit( $amp_url ) . user_trailingslashit( amp_get_slug(), 'single_amp' ); + if ( ! empty( $parsed_url['fragment'] ) ) { + $amp_url .= '#' . $parsed_url['fragment']; + } + } - if ( $post instanceof WP_Post ) { + if ( $post_id ) { /** * Filters AMP permalink. * * @since 0.2 - * @since 1.0 This filter does not apply when 'amp' theme support is present. - * @since 2.1 This filter applies again when in non-legacy Reader mode but only when obtaining an AMP URL for a post. - * @todo Deprecate this filter? + * @since 1.0 This filter only applies when using the legacy reader permalink structure. * - * @param false $amp_url AMP URL. - * @param int $post_id Post ID. + * @param string $amp_url AMP URL. + * @param int $post_id Post ID. */ - $amp_url = apply_filters( 'amp_get_permalink', $amp_url, $post->ID ); + $amp_url = apply_filters( 'amp_get_permalink', $amp_url, $post_id ); } return $amp_url; From 6ede6bdc48b3c19c23775b6a4df49e6acf7166b2 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Nov 2020 20:53:34 -0800 Subject: [PATCH 007/169] Add filters for implementing custom paired AMP permalink structures --- src/PairedAmpRouting.php | 132 ++++++++++++++++++++++++++++++--------- 1 file changed, 103 insertions(+), 29 deletions(-) diff --git a/src/PairedAmpRouting.php b/src/PairedAmpRouting.php index 16c2b07560a..e39199e4347 100644 --- a/src/PairedAmpRouting.php +++ b/src/PairedAmpRouting.php @@ -66,6 +66,15 @@ final class PairedAmpRouting implements Service, Registerable, Activateable, Dea */ const PERMALINK_STRUCTURE_LEGACY_READER = 'legacy_reader'; + /** + * Custom permalink structure. + * + * This involves a site adding the necessary filters to implement their own permalink structure. + * + * @var string + */ + const PERMALINK_STRUCTURE_CUSTOM = 'custom'; + /** * Permalink structures. * @@ -76,6 +85,7 @@ final class PairedAmpRouting implements Service, Registerable, Activateable, Dea self::PERMALINK_STRUCTURE_REWRITE_ENDPOINT, self::PERMALINK_STRUCTURE_LEGACY_TRANSITIONAL, self::PERMALINK_STRUCTURE_LEGACY_READER, + self::PERMALINK_STRUCTURE_CUSTOM, ]; /** @@ -195,16 +205,39 @@ public function sanitize_options( $options, $new_options ) { } /** - * Turn a given URL into a paired AMP URL. + * Get the current paired AMP permalink structure. * - * @todo Check if has_filter( 'amp_add_paired_endpoint' ) to defer to custom endpoint. + * @return string Paired AMP permalink structure. + */ + public function get_permalink_structure() { + $has_filters = [ + has_filter( 'amp_has_paired_endpoint' ), + has_filter( 'amp_add_paired_endpoint' ), + has_filter( 'amp_remove_paired_endpoint' ), + ]; + $has_filter_count = count( array_filter( $has_filters ) ); + if ( 3 === $has_filter_count ) { + return self::PERMALINK_STRUCTURE_CUSTOM; + } elseif ( $has_filter_count > 0 ) { + _doing_it_wrong( + 'add_filter', + esc_html__( 'In order to implement a custom paired AMP permalink structure, you must add three filters:', 'amp' ) . ' amp_has_paired_endpoint, amp_add_paired_endpoint, amp_remove_paired_endpoint', + '2.1' + ); + } + + return AMP_Options_Manager::get_option( Option::PERMALINK_STRUCTURE ); + } + + /** + * Turn a given URL into a paired AMP URL. * * @param string $url URL. * @return string AMP URL. */ public function add_paired_endpoint( $url ) { - - switch ( AMP_Options_Manager::get_option( Option::PERMALINK_STRUCTURE ) ) { + $permalink_structure = self::get_permalink_structure(); + switch ( $permalink_structure ) { case self::PERMALINK_STRUCTURE_REWRITE_ENDPOINT: return $this->get_rewrite_endpoint_paired_amp_url( $url ); case self::PERMALINK_STRUCTURE_LEGACY_TRANSITIONAL: @@ -214,7 +247,21 @@ public function add_paired_endpoint( $url ) { } // This is the PERMALINK_STRUCTURE_QUERY_VAR case, the default. - return $this->get_query_var_paired_amp_url( $url ); + $amp_url = $this->get_query_var_paired_amp_url( $url ); + + if ( self::PERMALINK_STRUCTURE_CUSTOM === $permalink_structure ) { + /** + * Filters paired AMP URL to apply a custom permalink structure. + * + * @since 2.1 + * + * @param string $amp_url AMP URL. By default the AMP query var is added. + * @param string $url Original URL. + */ + $amp_url = apply_filters( 'amp_add_paired_endpoint', $amp_url, $url ); + } + + return $amp_url; } /** @@ -364,45 +411,59 @@ public function get_legacy_reader_paired_amp_url( $url ) { * * @param string $url URL to examine. If empty, will use the current URL. * @return bool True if the AMP query parameter is set with the required value, false if not. - * @global WP_Query $wp_query + * @global WP_Query $wp_the_query */ public function has_paired_endpoint( $url = '' ) { $slug = amp_get_slug(); // If the URL was not provided, then use the environment which is already parsed. if ( empty( $url ) ) { - global $wp_query; - return ( + global $wp_the_query; + $has_endpoint = ( isset( $_GET[ $slug ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended || ( - $wp_query instanceof WP_Query + $wp_the_query instanceof WP_Query && - false !== $wp_query->get( $slug, false ) + false !== $wp_the_query->get( $slug, false ) ) ); - } + } else { + $has_endpoint = false; + + $parsed_url = wp_parse_url( $url ); + if ( ! empty( $parsed_url['query'] ) ) { + $query_vars = []; + wp_parse_str( $parsed_url['query'], $query_vars ); + if ( isset( $query_vars[ $slug ] ) ) { + $has_endpoint = true; + } + } - $parsed_url = wp_parse_url( $url ); - if ( ! empty( $parsed_url['query'] ) ) { - $query_vars = []; - wp_parse_str( $parsed_url['query'], $query_vars ); - if ( isset( $query_vars[ $slug ] ) ) { - return true; + if ( ! $has_endpoint && ! empty( $parsed_url['path'] ) ) { + $pattern = sprintf( + '#/%s(/[^/^])?/?$#', + preg_quote( $slug, '#' ) + ); + if ( preg_match( $pattern, $parsed_url['path'] ) ) { + $has_endpoint = true; + } } } - if ( ! empty( $parsed_url['path'] ) ) { - $pattern = sprintf( - '#/%s(/[^/^])?/?$#', - preg_quote( $slug, '#' ) - ); - if ( preg_match( $pattern, $parsed_url['path'] ) ) { - return true; - } + if ( self::get_permalink_structure() === self::PERMALINK_STRUCTURE_CUSTOM ) { + /** + * Filters whether the URL has a paired AMP permalink structure. + * + * @since 2.1 + * + * @param bool $has_endpoint Had endpoint. By default true if the AMP query var or rewrite endpoint is present. + * @param string $url The URL. + */ + $has_endpoint = apply_filters( 'amp_has_paired_endpoint', $has_endpoint, $url ?: amp_get_current_url() ); } - return false; + return $has_endpoint; } /** @@ -415,7 +476,7 @@ public function remove_paired_endpoint( $url ) { $slug = amp_get_slug(); // Strip endpoint, including /amp/, /amp/amp/, /amp/foo/. - $url = preg_replace( + $non_amp_url = preg_replace( sprintf( ':(/%s(/[^/?#]+)?)+(?=/?(\?|#|$)):', preg_quote( $slug, ':' ) @@ -425,9 +486,22 @@ public function remove_paired_endpoint( $url ) { ); // Strip query var, including ?amp, ?amp=1, etc. - $url = remove_query_arg( $slug, $url ); + $non_amp_url = remove_query_arg( $slug, $non_amp_url ); - return $url; + $permalink_structure = self::get_permalink_structure(); + if ( self::PERMALINK_STRUCTURE_CUSTOM === $permalink_structure ) { + /** + * Filters paired AMP URL to remove a custom permalink structure. + * + * @since 2.1 + * + * @param string $non_amp_url AMP URL. By default the rewrite endpoint and query var is removed. + * @param string $url Original URL. + */ + $non_amp_url = apply_filters( 'amp_remove_paired_endpoint', $non_amp_url, $url ); + } + + return $non_amp_url; } /** From b544dde58358379aa64c6050bae0d9af69c475e6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Nov 2020 23:05:33 -0800 Subject: [PATCH 008/169] Fix ability to preview post using a custom paired AMP URL --- .../components/amp-preview-button.js | 19 ++++++++++------ assets/src/block-editor/store/selectors.js | 19 ++++++++++++---- .../src/block-editor/store/test/selectors.js | 22 ++++++++++++++----- includes/admin/class-amp-post-meta-box.php | 3 ++- 4 files changed, 45 insertions(+), 18 deletions(-) diff --git a/assets/src/block-editor/components/amp-preview-button.js b/assets/src/block-editor/components/amp-preview-button.js index 252a0f366c1..eaf5bbf5a52 100644 --- a/assets/src/block-editor/components/amp-preview-button.js +++ b/assets/src/block-editor/components/amp-preview-button.js @@ -11,7 +11,6 @@ import { compose } from '@wordpress/compose'; import { withDispatch, withSelect } from '@wordpress/data'; import { Component, createRef, renderToString } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; -import { addQueryArgs } from '@wordpress/url'; /** * Internal dependencies @@ -253,7 +252,6 @@ export default compose( [ withSelect( ( select, { forcePreviewLink, forceIsAutosaveable } ) => { const { getCurrentPostId, - getCurrentPostAttribute, getEditedPostAttribute, isEditedPostSaveable, isEditedPostAutosaveable, @@ -261,20 +259,27 @@ export default compose( [ } = select( 'core/editor' ); const { - getAmpSlug, + getAmpUrl, + getAmpPreviewLink, getErrorMessages, isStandardMode, } = select( 'amp/block-editor' ); - const queryArgs = {}; - queryArgs[ getAmpSlug() ] = 1; + const copyQueryArgs = ( source, destination ) => { + const sourceUrl = new URL( source ); + const destinationUrl = new URL( destination ); + for ( const [ key, value ] of sourceUrl.searchParams.entries() ) { + destinationUrl.searchParams.set( key, value ); + } + return destinationUrl.href; + }; const initialPreviewLink = getEditedPostPreviewLink(); - const previewLink = initialPreviewLink ? addQueryArgs( initialPreviewLink, queryArgs ) : undefined; + const previewLink = initialPreviewLink ? copyQueryArgs( initialPreviewLink, getAmpPreviewLink() ) : undefined; return { postId: getCurrentPostId(), - currentPostLink: addQueryArgs( getCurrentPostAttribute( 'link' ), queryArgs ), + currentPostLink: getAmpUrl(), previewLink: forcePreviewLink !== undefined ? forcePreviewLink : previewLink, isSaveable: isEditedPostSaveable(), isAutosaveable: forceIsAutosaveable || isEditedPostAutosaveable(), diff --git a/assets/src/block-editor/store/selectors.js b/assets/src/block-editor/store/selectors.js index 02153bf2273..ac917e506b9 100644 --- a/assets/src/block-editor/store/selectors.js +++ b/assets/src/block-editor/store/selectors.js @@ -32,12 +32,23 @@ export function getErrorMessages( state ) { } /** - * Returns the AMP slug used in the query var, like 'amp'. + * Returns the AMP preview link (URL). * * @param {Object} state The editor state. * - * @return {string} The slug for AMP, like 'amp'. + * @return {string} The AMP preview link URL. */ -export function getAmpSlug( state ) { - return state.ampSlug; +export function getAmpPreviewLink( state ) { + return state.ampPreviewLink; +} + +/** + * Returns the AMP URL. + * + * @param {Object} state The editor state. + * + * @return {string} The AMP URL. + */ +export function getAmpUrl( state ) { + return state.ampUrl; } diff --git a/assets/src/block-editor/store/test/selectors.js b/assets/src/block-editor/store/test/selectors.js index 08da44bd3d3..19eacf352dd 100644 --- a/assets/src/block-editor/store/test/selectors.js +++ b/assets/src/block-editor/store/test/selectors.js @@ -5,7 +5,8 @@ import { hasThemeSupport, isStandardMode, getErrorMessages, - getAmpSlug, + getAmpPreviewLink, + getAmpUrl, } from '../selectors'; describe( 'selectors', () => { @@ -34,12 +35,21 @@ describe( 'selectors', () => { } ); } ); - describe( 'getAmpSlug', () => { - it( 'should return the AMP slug', () => { - const slug = 'amp'; - const state = { ampSlug: slug }; + describe( 'getAmpUrl', () => { + it( 'should return the paired AMP url', () => { + const url = 'https://example.com/?amp=1'; + const state = { ampUrl: url }; - expect( getAmpSlug( state ) ).toStrictEqual( slug ); + expect( getAmpUrl( state ) ).toStrictEqual( url ); + } ); + } ); + + describe( 'getAmpPreviewLink', () => { + it( 'should return the AMP preview link', () => { + const url = 'https://example.com/?preview=true&=1'; + const state = { ampPreviewLink: url }; + + expect( getAmpPreviewLink( state ) ).toStrictEqual( url ); } ); } ); } ); diff --git a/includes/admin/class-amp-post-meta-box.php b/includes/admin/class-amp-post-meta-box.php index acae775f39f..d406ff2aa1d 100644 --- a/includes/admin/class-amp-post-meta-box.php +++ b/includes/admin/class-amp-post-meta-box.php @@ -266,7 +266,8 @@ function () { ); $data = [ - 'ampSlug' => amp_get_slug(), + 'ampUrl' => amp_is_canonical() ? null : amp_add_paired_endpoint( get_permalink( $post ) ), + 'ampPreviewLink' => amp_is_canonical() ? null : amp_add_paired_endpoint( get_preview_post_link( $post ) ), 'errorMessages' => $this->get_error_messages( $status_and_errors['errors'] ), 'hasThemeSupport' => ! amp_is_legacy(), 'isStandardMode' => amp_is_canonical(), From ffaeeaff22eb4d277890b9ea2a51e71952f24fa7 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 3 Nov 2020 20:31:22 -0800 Subject: [PATCH 009/169] Allow paired browsing to work for custom paired URLs --- assets/src/admin/paired-browsing/app.js | 70 +++++++++++++--------- includes/class-amp-theme-support.php | 6 +- includes/templates/amp-paired-browsing.php | 6 +- tests/php/test-class-amp-meta-box.php | 4 +- 4 files changed, 49 insertions(+), 37 deletions(-) diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index 821ec6a435c..8503576ef99 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -8,7 +8,7 @@ import { addQueryArgs, hasQueryArg, removeQueryArgs } from '@wordpress/url'; import './app.css'; const { app, history } = window; -const { ampSlug, noampQueryVar, noampMobile, ampPairedBrowsingQueryVar, documentTitlePrefix } = app; +const { noampQueryVar, noampMobile, ampPairedBrowsingQueryVar, documentTitlePrefix } = app; class PairedBrowsingApp { /** @@ -175,28 +175,13 @@ class PairedBrowsingApp { } /** - * Removes AMP related query variables from the supplied URL. + * Purge removable query vars from the supplied URL. * * @param {string} url URL string. * @return {string} Modified URL without any AMP related query variables. */ - removeAmpQueryVars( url ) { - return removeQueryArgs( url, ampSlug, noampQueryVar, ampPairedBrowsingQueryVar ); - } - - /** - * Adds the AMP query variable to the supplied URL. - * - * @param {string} url URL string. - * @return {string} Modified URL with the AMP query variable. - */ - addAmpQueryVar( url ) { - return addQueryArgs( - url, - { - [ ampSlug ]: '1', - }, - ); + purgeRemovableQueryVars( url ) { + return removeQueryArgs( url, noampQueryVar, ampPairedBrowsingQueryVar ); } /** @@ -236,6 +221,20 @@ class PairedBrowsingApp { return hasQueryArg( url, 'amp_validation_errors' ); } + /** + * Get non-AMP location from a window. + * + * @param {Window} win Window. + * @return {string|null} Non-AMP location. + */ + getWindowNonAmpLocation( win ) { + if ( this.documentIsAmp( win.document ) ) { + const canonicalLink = win.document.querySelector( 'head > link[rel=canonical]' ); + return canonicalLink ? canonicalLink.href : null; + } + return win.location.href; + } + /** * Registers the provided client window with its parent, so that it can be managed by it. * @@ -243,9 +242,22 @@ class PairedBrowsingApp { */ registerClientWindow( win ) { let oppositeWindow; + const isAmp = this.documentIsAmp( win.document ); + + const amphtmlLink = win.document.querySelector( 'head > link[rel=amphtml]' ); + const canonicalLink = win.document.querySelector( 'head > link[rel=canonical]' ); + + let ampUrl, nonAmpUrl; + if ( isAmp ) { + ampUrl = win.location.href; + nonAmpUrl = canonicalLink ? canonicalLink.href : null; + } else { + nonAmpUrl = win.location.href; + ampUrl = amphtmlLink ? amphtmlLink.href : null; + } if ( win === this.ampIframe.contentWindow ) { - if ( ! this.documentIsAmp( win.document ) ) { + if ( ! isAmp ) { if ( this.urlHasValidationErrorQueryVar( win.location.href ) ) { /* * If the AMP page has validation errors, mark the page as invalid so that the @@ -254,9 +266,9 @@ class PairedBrowsingApp { this.ampPageHasErrors = true; this.toggleDisconnectOverlay( this.ampIframe ); return; - } else if ( win.document.querySelector( 'head > link[rel=amphtml]' ) ) { + } else if ( ampUrl ) { // Force the AMP iframe to always have an AMP URL, if an AMP version is available. - win.location.replace( this.addAmpQueryVar( win.location.href ) ); + win.location.replace( ampUrl ); return; } @@ -277,8 +289,8 @@ class PairedBrowsingApp { oppositeWindow = this.nonAmpIframe.contentWindow; } else { // Force the non-AMP iframe to always have a non-AMP URL. - if ( this.documentIsAmp( win.document ) ) { - win.location.replace( this.removeAmpQueryVars( win.location.href ) ); + if ( isAmp ) { + win.location.replace( this.purgeRemovableQueryVars( nonAmpUrl ) ); return; } @@ -310,13 +322,13 @@ class PairedBrowsingApp { oppositeWindow && oppositeWindow.location && ( - this.removeAmpQueryVars( this.removeUrlHash( oppositeWindow.location.href ) ) !== - this.removeAmpQueryVars( this.removeUrlHash( win.location.href ) ) + this.purgeRemovableQueryVars( this.removeUrlHash( this.getWindowNonAmpLocation( oppositeWindow ) ) ) !== + this.purgeRemovableQueryVars( this.removeUrlHash( this.getWindowNonAmpLocation( win ) ) ) ) ) { const url = oppositeWindow === this.ampIframe.contentWindow - ? this.addAmpQueryVar( win.location.href ) - : this.removeAmpQueryVars( win.location.href ); + ? ampUrl + : nonAmpUrl; oppositeWindow.location.replace( url ); @@ -328,7 +340,7 @@ class PairedBrowsingApp { history.replaceState( {}, '', - this.addPairedBrowsingQueryVar( this.removeAmpQueryVars( win.location.href ) ), + this.addPairedBrowsingQueryVar( this.purgeRemovableQueryVars( win.location.href ) ), ); } } diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index eb23057231c..474aac9e235 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -2394,10 +2394,11 @@ static function( $tag, $handle ) { */ public static function get_paired_browsing_url( $url = null ) { if ( ! $url ) { - $url = wp_unslash( $_SERVER['REQUEST_URI'] ); + $url = amp_get_current_url(); } + $url = amp_remove_paired_endpoint( $url ); $url = remove_query_arg( - [ amp_get_slug(), QueryVar::NOAMP, AMP_Validated_URL_Post_Type::VALIDATE_ACTION, AMP_Validation_Manager::VALIDATION_ERROR_TERM_STATUS_QUERY_VAR ], + [ QueryVar::NOAMP, AMP_Validated_URL_Post_Type::VALIDATE_ACTION, AMP_Validation_Manager::VALIDATION_ERROR_TERM_STATUS_QUERY_VAR ], $url ); $url = add_query_arg( self::PAIRED_BROWSING_QUERY_VAR, '1', $url ); @@ -2474,7 +2475,6 @@ public static function serve_paired_browsing_experience( $template ) { 'amp-paired-browsing-app', 'app', [ - 'ampSlug' => amp_get_slug(), 'ampPairedBrowsingQueryVar' => self::PAIRED_BROWSING_QUERY_VAR, 'noampQueryVar' => QueryVar::NOAMP, 'noampMobile' => QueryVar::NOAMP_MOBILE, diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index 461fcd5b576..fdcd590f4e2 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -7,7 +7,7 @@ use AmpProject\AmpWP\QueryVar; -$url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ] ); +$url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ], amp_get_current_url() ); $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, $url ); $amp_url = amp_add_paired_endpoint( $url ); ?> @@ -64,7 +64,7 @@ @@ -73,7 +73,7 @@ diff --git a/tests/php/test-class-amp-meta-box.php b/tests/php/test-class-amp-meta-box.php index f963d0642aa..ab39069a603 100644 --- a/tests/php/test-class-amp-meta-box.php +++ b/tests/php/test-class-amp-meta-box.php @@ -152,7 +152,6 @@ public function test_enqueue_block_assets() { 'wp-i18n', 'wp-plugins', 'wp-polyfill', - 'wp-url', ], $block_script->deps ); @@ -161,7 +160,8 @@ public function test_enqueue_block_assets() { $this->assertContains( 'ampBlockEditor', $block_script->extra['data'] ); $expected_localized_values = [ - 'ampSlug', + 'ampUrl', + 'ampPreviewLink', 'errorMessages', 'hasThemeSupport', 'isStandardMode', From e53528efb122840e37f886dc49b77de06cd5421a Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 4 Nov 2020 21:32:30 -0800 Subject: [PATCH 010/169] Use postMessage for navigation and scrolling --- assets/src/admin/paired-browsing/app.js | 283 +++++++++++++++------ assets/src/admin/paired-browsing/client.js | 183 +++++++++++-- assets/src/admin/paired-browsing/utils.js | 20 ++ includes/amp-helper-functions.php | 3 + includes/class-amp-theme-support.php | 16 +- includes/templates/amp-paired-browsing.php | 4 +- 6 files changed, 404 insertions(+), 105 deletions(-) create mode 100644 assets/src/admin/paired-browsing/utils.js diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index 8503576ef99..62c35a08800 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -6,9 +6,15 @@ import { addQueryArgs, hasQueryArg, removeQueryArgs } from '@wordpress/url'; * Internal dependencies */ import './app.css'; +import { isNonAmpWindow, isAmpWindow } from './utils'; -const { app, history } = window; -const { noampQueryVar, noampMobile, ampPairedBrowsingQueryVar, documentTitlePrefix } = app; +const { ampPairedBrowsingAppData, history } = window; +const { + noampQueryVar, + noampMobile, + ampPairedBrowsingQueryVar, + documentTitlePrefix, +} = ampPairedBrowsingAppData; class PairedBrowsingApp { /** @@ -25,6 +31,13 @@ class PairedBrowsingApp { */ ampIframe; + /** + * Whether the AMP window is loading. + * + * @type {boolean} + */ + ampWindowLoading = false; + /** * Non-AMP IFrame * @@ -32,6 +45,27 @@ class PairedBrowsingApp { */ nonAmpIframe; + /** + * Whether the non-AMP window is loading. + * + * @type {boolean} + */ + nonAmpWindowLoading = false; + + /** + * Current AMP URL. + * + * @type {string} + */ + currentAmpUrl; + + /** + * Current non-AMP URL. + * + * @type {string} + */ + currentNonAmpUrl; + /** * Non-AMP Link * @@ -46,13 +80,23 @@ class PairedBrowsingApp { */ ampLink; + /** + * Active iframe. + * + * @type {HTMLIFrameElement|null} + */ + activeIframe; + /** * Constructor. */ constructor() { this.nonAmpIframe = document.querySelector( '#non-amp iframe' ); this.ampIframe = document.querySelector( '#amp iframe' ); - this.ampPageHasErrors = false; + this.ampPageHasErrors = false; // @todo This is obsolete. Remove this and invalid-amp span. + + this.currentNonAmpUrl = this.nonAmpIframe.src; + this.currentAmpUrl = this.ampIframe.src; // Link to exit paired browsing. this.nonAmpLink = /** @type {HTMLAnchorElement} */ document.getElementById( 'non-amp-link' ); @@ -70,12 +114,71 @@ class PairedBrowsingApp { }; this.addDisconnectButtonListeners(); + window.addEventListener( 'message', ( event ) => { + this.receiveMessage( event ); + } ); + + // Set the active iframe based on which got the last mouseenter. + // Note that setting activeIframe may get set by receiveScroll if the user starts scrolling + // before moving the mouse. + document.getElementById( 'non-amp' ).addEventListener( 'mouseenter', () => { + this.activeIframe = this.nonAmpIframe; + } ); + document.getElementById( 'amp' ).addEventListener( 'mouseenter', () => { + this.activeIframe = this.ampIframe; + } ); + // Load clients. - Promise.all( this.getIframeLoadedPromises() ); + this.getIframeLoadedPromises(); + } + + /** + * Send message to app. + * + * @param {Window} win Window. + * @param {string} type Type. + * @param {Object} data Data. + */ + sendMessage( win, type, data = {} ) { + win.postMessage( + { + type, + ...data, + ampPairedBrowsing: true, + }, + isAmpWindow( win ) ? this.currentAmpUrl : this.currentNonAmpUrl, + ); + } + + /** + * Receive message. + * + * @param {MessageEvent} event + */ + receiveMessage( event ) { + if ( ! event.data || ! event.data.type || ! event.data.ampPairedBrowsing || ! event.source ) { + return; + } + + if ( ! isAmpWindow( event.source ) && ! isNonAmpWindow( event.source ) ) { + return; + } + + switch ( event.data.type ) { + case 'heartbeat': + this.receiveHeartbeat( event.data, event.source ); + break; + case 'scroll': + this.receiveScroll( event.data, event.source ); + break; + default: + } } /** * Add event listeners for buttons on disconnect overlay. + * + * @todo Revisit. */ addDisconnectButtonListeners() { // The 'Exit' button navigates the parent window to the URL of the disconnected client. @@ -98,33 +201,24 @@ class PairedBrowsingApp { return [ new Promise( ( resolve ) => { this.nonAmpIframe.addEventListener( 'load', () => { - this.toggleDisconnectOverlay( this.nonAmpIframe ); + //this.toggleDisconnectOverlay( this.nonAmpIframe ); resolve(); } ); } ), new Promise( ( resolve ) => { this.ampIframe.addEventListener( 'load', () => { - this.toggleDisconnectOverlay( this.ampIframe ); + //this.toggleDisconnectOverlay( this.ampIframe ); resolve(); } ); } ), ]; } - /** - * Validates whether or not the window document is AMP compatible. - * - * @param {Document} doc Window document. - * @return {boolean} True if AMP compatible, false if not. - */ - documentIsAmp( doc ) { - return doc.querySelector( 'head > script[src="https://cdn.ampproject.org/v0.js"]' ); - } - /** * Toggles the 'disconnected' overlay for the supplied iframe. * + * @todo Revisit. * @param {HTMLIFrameElement} iframe The iframe that hosts the paired browsing client. */ toggleDisconnectOverlay( iframe ) { @@ -162,6 +256,7 @@ class PairedBrowsingApp { /** * Determines the status of the paired browsing client in an iframe. * + * @todo Revisit. * @param {HTMLIFrameElement} iframe The iframe. */ isClientConnected( iframe ) { @@ -222,43 +317,76 @@ class PairedBrowsingApp { } /** - * Get non-AMP location from a window. + * Replace location. * - * @param {Window} win Window. - * @return {string|null} Non-AMP location. + * @param {HTMLIFrameElement} iframe IFrame Element. + * @param {string} url URL. */ - getWindowNonAmpLocation( win ) { - if ( this.documentIsAmp( win.document ) ) { - const canonicalLink = win.document.querySelector( 'head > link[rel=canonical]' ); - return canonicalLink ? canonicalLink.href : null; + replaceLocation( iframe, url ) { + // @todo If disconneted we canot send the replaceLocation message. + + if ( iframe === this.ampIframe ) { + this.ampWindowLoading = true; + } else { + this.nonAmpWindowLoading = true; } - return win.location.href; + + this.sendMessage( + iframe.contentWindow, + 'replaceLocation', + { href: url }, + ); } /** - * Registers the provided client window with its parent, so that it can be managed by it. + * Receive scroll. * - * @param {Window} win Document window. + * @param {Object} data Data. + * @param {boolean} data.x X position. + * @param {string|null} data.y Y position. + * @param {Window} sourceWindow The source window. */ - registerClientWindow( win ) { - let oppositeWindow; - const isAmp = this.documentIsAmp( win.document ); - - const amphtmlLink = win.document.querySelector( 'head > link[rel=amphtml]' ); - const canonicalLink = win.document.querySelector( 'head > link[rel=canonical]' ); + receiveScroll( { x, y }, sourceWindow ) { + // Rely on scroll event to determine initially-active iframe before mouse first moves. + if ( ! this.activeIframe ) { + this.activeIframe = isAmpWindow( sourceWindow ) + ? this.ampIframe + : this.nonAmpIframe; + } - let ampUrl, nonAmpUrl; - if ( isAmp ) { - ampUrl = win.location.href; - nonAmpUrl = canonicalLink ? canonicalLink.href : null; - } else { - nonAmpUrl = win.location.href; - ampUrl = amphtmlLink ? amphtmlLink.href : null; + // Ignore scroll events from the non-active iframe. + if ( ! this.activeIframe || sourceWindow !== this.activeIframe.contentWindow ) { + return; } - if ( win === this.ampIframe.contentWindow ) { - if ( ! isAmp ) { - if ( this.urlHasValidationErrorQueryVar( win.location.href ) ) { + const otherWindow = isAmpWindow( sourceWindow ) + ? this.nonAmpIframe.contentWindow + : this.ampIframe.contentWindow; + this.sendMessage( otherWindow, 'scroll', { x, y } ); + } + + /** + * Receive heartbeat. + * + * @param {Object} data Data. + * @param {boolean} data.isAmpDocument Whether the document is actually an AMP page. + * @param {string|null} data.ampUrl The AMP URL. + * @param {string|null} data.nonAmpUrl The non-AMP URL. + * @param {string} data.documentTitle The canonical link URL if present. + * @param {Window} sourceWindow The source window. + */ + receiveHeartbeat( { isAmpDocument, ampUrl, nonAmpUrl, documentTitle }, sourceWindow ) { + const isAmpSource = isAmpWindow( sourceWindow ); + const sourceIframe = isAmpSource ? this.ampIframe : this.nonAmpIframe; + + if ( isAmpSource ) { + // Stop if the URL has not changed. + if ( this.currentAmpUrl === ampUrl ) { + return; + } + + if ( ! isAmpDocument ) { + if ( this.urlHasValidationErrorQueryVar( ampUrl ) ) { /* * If the AMP page has validation errors, mark the page as invalid so that the * 'disconnected' overlay can be shown. @@ -268,7 +396,7 @@ class PairedBrowsingApp { return; } else if ( ampUrl ) { // Force the AMP iframe to always have an AMP URL, if an AMP version is available. - win.location.replace( ampUrl ); + this.replaceLocation( sourceIframe, ampUrl ); return; } @@ -278,69 +406,68 @@ class PairedBrowsingApp { * overlay. */ this.ampPageHasErrors = true; - this.toggleDisconnectOverlay( this.ampIframe ); + // this.toggleDisconnectOverlay( this.ampIframe ); return; } + this.currentAmpUrl = ampUrl; + // Update the AMP link above the iframe used for exiting paired browsing. - this.ampLink.href = removeQueryArgs( this.ampIframe.contentWindow.location.href, noampQueryVar ); + this.ampLink.href = removeQueryArgs( ampUrl, noampQueryVar ); this.ampPageHasErrors = false; - oppositeWindow = this.nonAmpIframe.contentWindow; } else { + // Stop if the URL has not changed. + if ( this.currentNonAmpUrl === nonAmpUrl ) { + return; + } + // Force the non-AMP iframe to always have a non-AMP URL. - if ( isAmp ) { - win.location.replace( this.purgeRemovableQueryVars( nonAmpUrl ) ); + if ( isAmpDocument ) { + this.replaceLocation( sourceIframe, nonAmpUrl ); return; } + this.currentNonAmpUrl = nonAmpUrl; + // Update the non-AMP link above the iframe used for exiting paired browsing. this.nonAmpLink.href = addQueryArgs( - this.nonAmpIframe.contentWindow.location.href, + nonAmpUrl, { [ noampQueryVar ]: noampMobile }, ); - - oppositeWindow = this.ampIframe.contentWindow; } - // Synchronize scrolling from current window to its opposite. - win.addEventListener( - 'scroll', - () => { - if ( oppositeWindow && oppositeWindow.ampPairedBrowsingClient && oppositeWindow.scrollTo ) { - oppositeWindow.scrollTo( win.scrollX, win.scrollY ); - } - }, - { passive: true }, - ); - - // Scrolling is not synchronized if `scroll-behavior` is set to `smooth`. - win.document.documentElement.style.setProperty( 'scroll-behavior', 'auto', 'important' ); - // Make sure the opposite iframe is set to match. + const thisCurrentUrl = isAmpSource ? nonAmpUrl : ampUrl; + const otherCurrentUrl = isAmpSource ? this.currentNonAmpUrl : this.currentAmpUrl; + if ( - oppositeWindow && - oppositeWindow.location && - ( - this.purgeRemovableQueryVars( this.removeUrlHash( this.getWindowNonAmpLocation( oppositeWindow ) ) ) !== - this.purgeRemovableQueryVars( this.removeUrlHash( this.getWindowNonAmpLocation( win ) ) ) - ) + this.purgeRemovableQueryVars( this.removeUrlHash( thisCurrentUrl ) ) !== + this.purgeRemovableQueryVars( this.removeUrlHash( otherCurrentUrl ) ) ) { - const url = oppositeWindow === this.ampIframe.contentWindow - ? ampUrl - : nonAmpUrl; - - oppositeWindow.location.replace( url ); + const url = isAmpSource + ? nonAmpUrl + : ampUrl; + this.replaceLocation( + isAmpSource ? this.nonAmpIframe : this.ampIframe, + this.purgeRemovableQueryVars( url ), + ); return; } - document.title = documentTitlePrefix + ' ' + win.document.title; + if ( isAmpSource ) { + this.ampWindowLoading = false; + } else { + this.nonAmpWindowLoading = false; + } + + document.title = documentTitlePrefix + ' ' + documentTitle; history.replaceState( {}, '', - this.addPairedBrowsingQueryVar( this.purgeRemovableQueryVars( win.location.href ) ), + this.addPairedBrowsingQueryVar( this.purgeRemovableQueryVars( nonAmpUrl ) ), ); } } diff --git a/assets/src/admin/paired-browsing/client.js b/assets/src/admin/paired-browsing/client.js index c54c8779f0f..646a155c33f 100644 --- a/assets/src/admin/paired-browsing/client.js +++ b/assets/src/admin/paired-browsing/client.js @@ -3,33 +3,168 @@ */ import domReady from '@wordpress/dom-ready'; -const { parent } = window; +/** + * Internal dependencies + */ +import { isNonAmpWindow, isAmpWindow } from './utils'; -if ( parent.pairedBrowsingApp ) { - window.ampPairedBrowsingClient = true; - const app = parent.pairedBrowsingApp; +const { parent, ampPairedBrowsingClientData } = window; +const { ampUrl, nonAmpUrl } = ampPairedBrowsingClientData; - app.registerClientWindow( window ); +/** + * Validates whether or not the window document is AMP compatible. + * + * @return {boolean} True if AMP compatible, false if not. + */ +function documentIsAmp() { + return Boolean( document.querySelector( 'head > script[src="https://cdn.ampproject.org/v0.js"]' ) ); +} - domReady( () => { - if ( app.documentIsAmp( document ) ) { - // Hide the paired browsing menu item. - const pairedBrowsingMenuItem = document.getElementById( 'wp-admin-bar-amp-paired-browsing' ); - if ( pairedBrowsingMenuItem ) { - pairedBrowsingMenuItem.remove(); - } - - // Hide menu item to view non-AMP version. - const ampViewBrowsingItem = document.getElementById( 'wp-admin-bar-amp-view' ); - if ( ampViewBrowsingItem ) { - ampViewBrowsingItem.remove(); - } - } else { - /** - * No need to show the AMP menu in the Non-AMP window. - */ - const ampMenuItem = document.getElementById( 'wp-admin-bar-amp' ); - ampMenuItem.remove(); +// /** +// * Get amphtml link URL. +// * +// * @return {string|null} URL or null if link not present. +// */ +// function getAmphtmlLinkHref() { +// const link = document.querySelector( 'head > link[rel=amphtml]' ); +// return link ? link.href : null; +// } +// +// /** +// * Get canonical link URL. +// * +// * @return {string|null} URL or null if link not present. +// */ +// function getCanonicalLinkHref() { +// const link = document.querySelector( 'head > link[rel=canonical]' ); +// return link ? link.href : null; +// } + +/** + * Modify document for paired browsing. + */ +function modifyDocumentForPairedBrowsing() { + // Scrolling is not synchronized if `scroll-behavior` is set to `smooth`. + document.documentElement.style.setProperty( 'scroll-behavior', 'auto', 'important' ); + + if ( documentIsAmp() ) { + // Hide the paired browsing menu item. + const pairedBrowsingMenuItem = document.getElementById( 'wp-admin-bar-amp-paired-browsing' ); + if ( pairedBrowsingMenuItem ) { + pairedBrowsingMenuItem.remove(); } + + // Hide menu item to view non-AMP version. + const ampViewBrowsingItem = document.getElementById( 'wp-admin-bar-amp-view' ); + if ( ampViewBrowsingItem ) { + ampViewBrowsingItem.remove(); + } + } else { + // No need to show the AMP menu in the Non-AMP window. + const ampMenuItem = document.getElementById( 'wp-admin-bar-amp' ); + ampMenuItem.remove(); + } +} + +/** + * Send message to app. + * + * @param {Window} win Window. + * @param {string} type Type. + * @param {Object} data Data. + */ +function sendMessage( win, type, data ) { + win.postMessage( + { + type, + ...data, + ampPairedBrowsing: true, + }, + nonAmpUrl, // Because the paired browsing app is accessed via the canonical URL. + ); +} + +/** + * Receive message. + * + * @param {MessageEvent} event + */ +function receiveMessage( event ) { + if ( ! event.data || ! event.data.ampPairedBrowsing || ! event.data.type || ! event.source ) { + return; + } + switch ( event.data.type ) { + case 'scroll': + receiveScroll( event.data ); + break; + case 'replaceLocation': + receiveReplaceLocation( event.data ); + break; + default: + } +} + +/** + * Send scroll. + */ +function sendScroll() { + sendMessage( + parent, + 'scroll', + { + x: window.scrollX, + y: window.scrollY, + }, + ); +} + +/** + * Receive scroll. + * + * @param {Object} data + * @param {number} data.x + * @param {number} data.y + */ +function receiveScroll( { x, y } ) { + window.scrollTo( x, y ); +} + +/** + * Receive replace location. + * + * @param {string} href + */ +function receiveReplaceLocation( { href } ) { + window.location.replace( href ); +} + +/** + * Send heartbeat. + * + * @see https://github.com/WordPress/wordpress-develop/blob/7a16c4d5809507bbfa9eb0f95178092492b04727/src/js/_enqueues/wp/customize/controls.js#L6679-L6727 + */ +function sendHeartbeat() { + sendMessage( + parent, + 'heartbeat', + { + isAmpDocument: documentIsAmp(), + //locationHref: window.location.href, + ampUrl, + nonAmpUrl, + documentTitle: document.title, + }, + ); +} + +if ( isNonAmpWindow( window ) || isAmpWindow( window ) ) { + domReady( () => { + modifyDocumentForPairedBrowsing(); + + window.addEventListener( 'message', receiveMessage ); + window.addEventListener( 'scroll', sendScroll, { passive: true } ); + + sendHeartbeat(); + setInterval( sendHeartbeat, 1000 ); } ); } diff --git a/assets/src/admin/paired-browsing/utils.js b/assets/src/admin/paired-browsing/utils.js new file mode 100644 index 00000000000..87cba9e8658 --- /dev/null +++ b/assets/src/admin/paired-browsing/utils.js @@ -0,0 +1,20 @@ + +/** + * Return whether the window is for the non-AMP page. + * + * @param {Window} win Window. + * @return {boolean} Whether non-AMP window. + */ +export function isNonAmpWindow( win ) { + return win.name === 'paired-browsing-non-amp'; +} + +/** + * Return whether the window is for the AMP page. + * + * @param {Window} win Window. + * @return {boolean} Whether AMP window. + */ +export function isAmpWindow( win ) { + return win.name === 'paired-browsing-amp'; +} diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index cabde0e6f7f..d3b03c08aed 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1432,6 +1432,9 @@ function amp_get_content_sanitizers( $post = null ) { */ $dev_mode_xpaths = (array) apply_filters( 'amp_dev_mode_element_xpaths', [] ); + // Paired browsing paths. + $dev_mode_xpaths[] = '//script[ contains( text(), "ampPairedBrowsingClient" ) ]'; + if ( is_admin_bar_showing() ) { $dev_mode_xpaths[] = '//*[ @id = "wpadminbar" ]'; $dev_mode_xpaths[] = '//*[ @id = "wpadminbar" ]//*'; diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 474aac9e235..4841d7702d7 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -2369,6 +2369,20 @@ public static function setup_paired_browsing_client() { true ); + $is_amp_request = amp_is_request(); + $current_url = amp_get_current_url(); + $amp_url = $is_amp_request ? $current_url : amp_add_paired_endpoint( $current_url ); + $non_amp_url = ! $is_amp_request ? $current_url : amp_remove_paired_endpoint( $current_url ); + + wp_localize_script( + 'amp-paired-browsing-client', + 'ampPairedBrowsingClientData', + [ + 'ampUrl' => $amp_url, + 'nonAmpUrl' => $non_amp_url, + ] + ); + // Mark enqueued script for AMP dev mode so that it is not removed. // @todo Revisit with . add_filter( @@ -2473,7 +2487,7 @@ public static function serve_paired_browsing_experience( $template ) { wp_localize_script( 'amp-paired-browsing-app', - 'app', + 'ampPairedBrowsingAppData', [ 'ampPairedBrowsingQueryVar' => self::PAIRED_BROWSING_QUERY_VAR, 'noampQueryVar' => QueryVar::NOAMP, diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index fdcd590f4e2..f3807ec6e28 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -64,7 +64,7 @@ @@ -73,7 +73,7 @@ From 65eaa08b6541a7439928ebfec6ab43cc877900c5 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 4 Nov 2020 21:34:41 -0800 Subject: [PATCH 011/169] Remove unused vars --- assets/src/admin/paired-browsing/app.js | 26 ------------------------- 1 file changed, 26 deletions(-) diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index 62c35a08800..f52dac52365 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -31,13 +31,6 @@ class PairedBrowsingApp { */ ampIframe; - /** - * Whether the AMP window is loading. - * - * @type {boolean} - */ - ampWindowLoading = false; - /** * Non-AMP IFrame * @@ -45,13 +38,6 @@ class PairedBrowsingApp { */ nonAmpIframe; - /** - * Whether the non-AMP window is loading. - * - * @type {boolean} - */ - nonAmpWindowLoading = false; - /** * Current AMP URL. * @@ -325,12 +311,6 @@ class PairedBrowsingApp { replaceLocation( iframe, url ) { // @todo If disconneted we canot send the replaceLocation message. - if ( iframe === this.ampIframe ) { - this.ampWindowLoading = true; - } else { - this.nonAmpWindowLoading = true; - } - this.sendMessage( iframe.contentWindow, 'replaceLocation', @@ -456,12 +436,6 @@ class PairedBrowsingApp { return; } - if ( isAmpSource ) { - this.ampWindowLoading = false; - } else { - this.nonAmpWindowLoading = false; - } - document.title = documentTitlePrefix + ' ' + documentTitle; history.replaceState( From 5d4f267f502cbd4444850d8815ae776c5034733a Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 4 Nov 2020 21:52:06 -0800 Subject: [PATCH 012/169] Remove obsolete ampPageHasErrors and improve isAmpDocument passing --- assets/src/admin/paired-browsing/app.js | 53 ++-------------------- assets/src/admin/paired-browsing/client.js | 36 ++------------- includes/class-amp-theme-support.php | 5 +- includes/templates/amp-paired-browsing.php | 4 -- 4 files changed, 11 insertions(+), 87 deletions(-) diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index f52dac52365..0b048b55e60 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -1,7 +1,7 @@ /** * WordPress dependencies */ -import { addQueryArgs, hasQueryArg, removeQueryArgs } from '@wordpress/url'; +import { addQueryArgs, removeQueryArgs } from '@wordpress/url'; /** * Internal dependencies */ @@ -79,7 +79,6 @@ class PairedBrowsingApp { constructor() { this.nonAmpIframe = document.querySelector( '#non-amp iframe' ); this.ampIframe = document.querySelector( '#amp iframe' ); - this.ampPageHasErrors = false; // @todo This is obsolete. Remove this and invalid-amp span. this.currentNonAmpUrl = this.nonAmpIframe.src; this.currentAmpUrl = this.ampIframe.src; @@ -90,10 +89,6 @@ class PairedBrowsingApp { // Overlay that is displayed on the client that becomes disconnected. this.disconnectOverlay = document.querySelector( '.disconnect-overlay' ); - this.disconnectText = { - general: document.querySelector( '.disconnect-overlay .dialog-text span.general' ), - invalidAmp: document.querySelector( '.disconnect-overlay .dialog-text span.invalid-amp' ), - }; this.disconnectButtons = { exit: document.querySelector( '.disconnect-overlay .button.exit' ), goBack: document.querySelector( '.disconnect-overlay .button.go-back' ), @@ -211,16 +206,9 @@ class PairedBrowsingApp { const isClientConnected = this.isClientConnected( iframe ); if ( ! isClientConnected ) { - if ( this.ampIframe === iframe && this.ampPageHasErrors ) { - this.disconnectText.general.classList.toggle( 'hidden', true ); - this.disconnectText.invalidAmp.classList.toggle( 'hidden', false ); - } else { - this.disconnectText.general.classList.toggle( 'hidden', false ); - this.disconnectText.invalidAmp.classList.toggle( 'hidden', true ); - } - // Show the 'Go Back' button if the parent window has history. this.disconnectButtons.goBack.classList.toggle( 'hidden', 0 >= window.history.length ); + // If the document is not available, the window URL cannot be accessed. this.disconnectButtons.exit.classList.toggle( 'hidden', null === iframe.contentDocument ); @@ -246,7 +234,7 @@ class PairedBrowsingApp { * @param {HTMLIFrameElement} iframe The iframe. */ isClientConnected( iframe ) { - if ( this.ampIframe === iframe && this.ampPageHasErrors ) { + if ( this.ampIframe === iframe ) { return false; } @@ -292,16 +280,6 @@ class PairedBrowsingApp { return parsedUrl.href; } - /** - * Checks if a URL has the 'amp_validation_errors' query variable. - * - * @param {string} url URL string. - * @return {boolean} True if such query var exists, false if not. - */ - urlHasValidationErrorQueryVar( url ) { - return hasQueryArg( url, 'amp_validation_errors' ); - } - /** * Replace location. * @@ -365,28 +343,9 @@ class PairedBrowsingApp { return; } + // Force the AMP iframe to always have an AMP URL. if ( ! isAmpDocument ) { - if ( this.urlHasValidationErrorQueryVar( ampUrl ) ) { - /* - * If the AMP page has validation errors, mark the page as invalid so that the - * 'disconnected' overlay can be shown. - */ - this.ampPageHasErrors = true; - this.toggleDisconnectOverlay( this.ampIframe ); - return; - } else if ( ampUrl ) { - // Force the AMP iframe to always have an AMP URL, if an AMP version is available. - this.replaceLocation( sourceIframe, ampUrl ); - return; - } - - /* - * If the AMP iframe has loaded a non-AMP page and none of the conditions above are - * true, then explicitly mark it as having errors and display the 'disconnected - * overlay. - */ - this.ampPageHasErrors = true; - // this.toggleDisconnectOverlay( this.ampIframe ); + this.replaceLocation( sourceIframe, ampUrl ); return; } @@ -394,8 +353,6 @@ class PairedBrowsingApp { // Update the AMP link above the iframe used for exiting paired browsing. this.ampLink.href = removeQueryArgs( ampUrl, noampQueryVar ); - - this.ampPageHasErrors = false; } else { // Stop if the URL has not changed. if ( this.currentNonAmpUrl === nonAmpUrl ) { diff --git a/assets/src/admin/paired-browsing/client.js b/assets/src/admin/paired-browsing/client.js index 646a155c33f..c1afebdcf1c 100644 --- a/assets/src/admin/paired-browsing/client.js +++ b/assets/src/admin/paired-browsing/client.js @@ -9,36 +9,7 @@ import domReady from '@wordpress/dom-ready'; import { isNonAmpWindow, isAmpWindow } from './utils'; const { parent, ampPairedBrowsingClientData } = window; -const { ampUrl, nonAmpUrl } = ampPairedBrowsingClientData; - -/** - * Validates whether or not the window document is AMP compatible. - * - * @return {boolean} True if AMP compatible, false if not. - */ -function documentIsAmp() { - return Boolean( document.querySelector( 'head > script[src="https://cdn.ampproject.org/v0.js"]' ) ); -} - -// /** -// * Get amphtml link URL. -// * -// * @return {string|null} URL or null if link not present. -// */ -// function getAmphtmlLinkHref() { -// const link = document.querySelector( 'head > link[rel=amphtml]' ); -// return link ? link.href : null; -// } -// -// /** -// * Get canonical link URL. -// * -// * @return {string|null} URL or null if link not present. -// */ -// function getCanonicalLinkHref() { -// const link = document.querySelector( 'head > link[rel=canonical]' ); -// return link ? link.href : null; -// } +const { ampUrl, nonAmpUrl, isAmpDocument } = ampPairedBrowsingClientData; /** * Modify document for paired browsing. @@ -47,7 +18,7 @@ function modifyDocumentForPairedBrowsing() { // Scrolling is not synchronized if `scroll-behavior` is set to `smooth`. document.documentElement.style.setProperty( 'scroll-behavior', 'auto', 'important' ); - if ( documentIsAmp() ) { + if ( isAmpDocument ) { // Hide the paired browsing menu item. const pairedBrowsingMenuItem = document.getElementById( 'wp-admin-bar-amp-paired-browsing' ); if ( pairedBrowsingMenuItem ) { @@ -148,8 +119,7 @@ function sendHeartbeat() { parent, 'heartbeat', { - isAmpDocument: documentIsAmp(), - //locationHref: window.location.href, + isAmpDocument, ampUrl, nonAmpUrl, documentTitle: document.title, diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 4841d7702d7..3dc9f7a06e1 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -2378,8 +2378,9 @@ public static function setup_paired_browsing_client() { 'amp-paired-browsing-client', 'ampPairedBrowsingClientData', [ - 'ampUrl' => $amp_url, - 'nonAmpUrl' => $non_amp_url, + 'isAmpDocument' => $is_amp_request, + 'ampUrl' => $amp_url, + 'nonAmpUrl' => $non_amp_url, ] ); diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index f3807ec6e28..63e9aa96be4 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -43,10 +43,6 @@ - - - -
From 4d8808b9d69b4dad2e8a383750fd278721897399 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 4 Nov 2020 22:56:57 -0800 Subject: [PATCH 013/169] Implement heartbeat check --- assets/src/admin/paired-browsing/app.js | 142 ++++++++++++--------- assets/src/admin/paired-browsing/client.js | 22 ++-- includes/templates/amp-paired-browsing.php | 3 - 3 files changed, 94 insertions(+), 73 deletions(-) diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index 0b048b55e60..4f563d8efe1 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -31,6 +31,13 @@ class PairedBrowsingApp { */ ampIframe; + /** + * Timestamp when the AMP iframe last sent a heartbeat. + * + * @type {number} + */ + ampHeartbeatTimestamp = Date.now(); + /** * Non-AMP IFrame * @@ -38,6 +45,13 @@ class PairedBrowsingApp { */ nonAmpIframe; + /** + * Timestamp when the non-AMP iframe last sent a heartbeat. + * + * @type {number} + */ + nonAmpHeartbeatTimestamp = Date.now(); + /** * Current AMP URL. * @@ -90,7 +104,6 @@ class PairedBrowsingApp { // Overlay that is displayed on the client that becomes disconnected. this.disconnectOverlay = document.querySelector( '.disconnect-overlay' ); this.disconnectButtons = { - exit: document.querySelector( '.disconnect-overlay .button.exit' ), goBack: document.querySelector( '.disconnect-overlay .button.go-back' ), }; this.addDisconnectButtonListeners(); @@ -110,7 +123,14 @@ class PairedBrowsingApp { } ); // Load clients. - this.getIframeLoadedPromises(); + Promise.all( this.getIframeLoadedPromises() ).then( () => { + setInterval( + () => { + this.checkConnectedClients(); + }, + 500, + ); + } ); } /** @@ -146,33 +166,19 @@ class PairedBrowsingApp { } switch ( event.data.type ) { - case 'heartbeat': - this.receiveHeartbeat( event.data, event.source ); + case 'loaded': + this.receiveLoaded( event.data, event.source ); break; case 'scroll': this.receiveScroll( event.data, event.source ); break; + case 'heartbeat': + this.receiveHeartbeat( event.source ); + break; default: } } - /** - * Add event listeners for buttons on disconnect overlay. - * - * @todo Revisit. - */ - addDisconnectButtonListeners() { - // The 'Exit' button navigates the parent window to the URL of the disconnected client. - this.disconnectButtons.exit.addEventListener( 'click', () => { - window.location.assign( this.disconnectedClient.contentWindow.location.href ); - } ); - - // The 'Go back' button goes back to the previous page of the parent window. - this.disconnectButtons.goBack.addEventListener( 'click', () => { - window.history.back(); - } ); - } - /** * Return promises to load iframes asynchronously. * @@ -181,66 +187,80 @@ class PairedBrowsingApp { getIframeLoadedPromises() { return [ new Promise( ( resolve ) => { - this.nonAmpIframe.addEventListener( 'load', () => { - //this.toggleDisconnectOverlay( this.nonAmpIframe ); - resolve(); - } ); + this.nonAmpIframe.addEventListener( 'load', resolve ); } ), - new Promise( ( resolve ) => { - this.ampIframe.addEventListener( 'load', () => { - //this.toggleDisconnectOverlay( this.ampIframe ); - resolve(); - } ); + this.ampIframe.addEventListener( 'load', resolve ); } ), ]; } /** - * Toggles the 'disconnected' overlay for the supplied iframe. + * Receive heartbeat. * - * @todo Revisit. - * @param {HTMLIFrameElement} iframe The iframe that hosts the paired browsing client. + * @param {Window} sourceWindow The source window. */ - toggleDisconnectOverlay( iframe ) { - const isClientConnected = this.isClientConnected( iframe ); + receiveHeartbeat( sourceWindow ) { + if ( isAmpWindow( sourceWindow ) ) { + this.ampHeartbeatTimestamp = Date.now(); + } else { + this.nonAmpHeartbeatTimestamp = Date.now(); + } + } - if ( ! isClientConnected ) { - // Show the 'Go Back' button if the parent window has history. - this.disconnectButtons.goBack.classList.toggle( 'hidden', 0 >= window.history.length ); + /** + * Check connected clients. + */ + checkConnectedClients() { + if ( ! this.isClientConnected( this.ampIframe ) ) { + this.showDisconnectOverlay( this.ampIframe ); + } else if ( ! this.isClientConnected( this.nonAmpIframe ) ) { + this.showDisconnectOverlay( this.nonAmpIframe ); + } else { + this.disconnectOverlay.classList.remove( 'disconnected' ); + } + } - // If the document is not available, the window URL cannot be accessed. - this.disconnectButtons.exit.classList.toggle( 'hidden', null === iframe.contentDocument ); + /** + * Add event listeners for buttons on disconnect overlay. + */ + addDisconnectButtonListeners() { + // The 'Go back' button goes back to the previous page of the parent window. + this.disconnectButtons.goBack.addEventListener( 'click', () => { + window.history.back(); + } ); + } - this.disconnectedClient = iframe; - } + /** + * Shows the 'disconnected' overlay for the supplied iframe. + * + * @param {HTMLIFrameElement} iframe The iframe that hosts the paired browsing client. + */ + showDisconnectOverlay( iframe ) { + // Show the 'Go Back' button if the parent window has history. + this.disconnectButtons.goBack.classList.toggle( 'hidden', 0 >= window.history.length ); // Applying the 'amp' class will overlay it on the AMP iframe. this.disconnectOverlay.classList.toggle( 'amp', - ! isClientConnected && this.ampIframe === iframe, + this.ampIframe === iframe, ); - this.disconnectOverlay.classList.toggle( - 'disconnected', - ! isClientConnected, - ); + this.disconnectOverlay.classList.add( 'disconnected' ); } /** * Determines the status of the paired browsing client in an iframe. * - * @todo Revisit. * @param {HTMLIFrameElement} iframe The iframe. + * @return {boolean} Whether the client is connected. */ isClientConnected( iframe ) { - if ( this.ampIframe === iframe ) { - return false; + const threshold = 2000; + if ( iframe === this.ampIframe ) { + return Date.now() - this.ampHeartbeatTimestamp < threshold; } - - return null !== iframe.contentWindow && - null !== iframe.contentDocument && - true === iframe.contentWindow.ampPairedBrowsingClient; + return Date.now() - this.nonAmpHeartbeatTimestamp < threshold; } /** @@ -287,8 +307,6 @@ class PairedBrowsingApp { * @param {string} url URL. */ replaceLocation( iframe, url ) { - // @todo If disconneted we canot send the replaceLocation message. - this.sendMessage( iframe.contentWindow, 'replaceLocation', @@ -299,10 +317,10 @@ class PairedBrowsingApp { /** * Receive scroll. * - * @param {Object} data Data. - * @param {boolean} data.x X position. - * @param {string|null} data.y Y position. - * @param {Window} sourceWindow The source window. + * @param {Object} data Data. + * @param {number} data.x X position. + * @param {number} data.y Y position. + * @param {Window} sourceWindow The source window. */ receiveScroll( { x, y }, sourceWindow ) { // Rely on scroll event to determine initially-active iframe before mouse first moves. @@ -324,7 +342,7 @@ class PairedBrowsingApp { } /** - * Receive heartbeat. + * Receive loaded. * * @param {Object} data Data. * @param {boolean} data.isAmpDocument Whether the document is actually an AMP page. @@ -333,7 +351,7 @@ class PairedBrowsingApp { * @param {string} data.documentTitle The canonical link URL if present. * @param {Window} sourceWindow The source window. */ - receiveHeartbeat( { isAmpDocument, ampUrl, nonAmpUrl, documentTitle }, sourceWindow ) { + receiveLoaded( { isAmpDocument, ampUrl, nonAmpUrl, documentTitle }, sourceWindow ) { const isAmpSource = isAmpWindow( sourceWindow ); const sourceIframe = isAmpSource ? this.ampIframe : this.nonAmpIframe; diff --git a/assets/src/admin/paired-browsing/client.js b/assets/src/admin/paired-browsing/client.js index c1afebdcf1c..225fb540370 100644 --- a/assets/src/admin/paired-browsing/client.js +++ b/assets/src/admin/paired-browsing/client.js @@ -44,7 +44,7 @@ function modifyDocumentForPairedBrowsing() { * @param {string} type Type. * @param {Object} data Data. */ -function sendMessage( win, type, data ) { +function sendMessage( win, type, data = {} ) { win.postMessage( { type, @@ -110,14 +110,12 @@ function receiveReplaceLocation( { href } ) { } /** - * Send heartbeat. - * - * @see https://github.com/WordPress/wordpress-develop/blob/7a16c4d5809507bbfa9eb0f95178092492b04727/src/js/_enqueues/wp/customize/controls.js#L6679-L6727 + * Send loaded. */ -function sendHeartbeat() { +function sendLoaded() { sendMessage( parent, - 'heartbeat', + 'loaded', { isAmpDocument, ampUrl, @@ -127,14 +125,22 @@ function sendHeartbeat() { ); } +/** + * Send heartbeat. + */ +function sendHeartbeat() { + sendMessage( parent, 'heartbeat' ); +} + if ( isNonAmpWindow( window ) || isAmpWindow( window ) ) { + setInterval( sendHeartbeat, 500 ); + domReady( () => { modifyDocumentForPairedBrowsing(); window.addEventListener( 'message', receiveMessage ); window.addEventListener( 'scroll', sendScroll, { passive: true } ); - sendHeartbeat(); - setInterval( sendHeartbeat, 1000 ); + sendLoaded(); } ); } diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index 63e9aa96be4..f6bdaaa0b6e 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -46,9 +46,6 @@
- From c7805afb73961e4aabcab7914eb4ddd96b4cc742 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Nov 2020 14:31:08 -0800 Subject: [PATCH 014/169] Restore exit link in modal --- assets/src/admin/paired-browsing/app.css | 9 ++-- assets/src/admin/paired-browsing/app.js | 56 +++++++++++++++++++--- assets/src/admin/paired-browsing/client.js | 17 +++++++ includes/templates/amp-paired-browsing.php | 5 +- 4 files changed, 72 insertions(+), 15 deletions(-) diff --git a/assets/src/admin/paired-browsing/app.css b/assets/src/admin/paired-browsing/app.css index 5e20cb7fc08..c9486064496 100644 --- a/assets/src/admin/paired-browsing/app.css +++ b/assets/src/admin/paired-browsing/app.css @@ -152,7 +152,10 @@ iframe { padding: 13px 16px; } -.disconnect-overlay .dialog .dialog-buttons button { +.disconnect-overlay .dialog .dialog-buttons .button { + text-decoration: none; + color: #000; + background: rgb(239, 239, 239); margin: 5px; border: none; box-shadow: none; @@ -189,7 +192,3 @@ iframe { height: auto; width: auto; } - -.hidden { - display: none; -} diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index 4f563d8efe1..1e4d03bea72 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -59,6 +59,13 @@ class PairedBrowsingApp { */ currentAmpUrl; + /** + * The most recent URL that was being navigated to in the AMP window. + * + * @type {?string} + */ + navigateAmpUrl; + /** * Current non-AMP URL. * @@ -66,6 +73,13 @@ class PairedBrowsingApp { */ currentNonAmpUrl; + /** + * The most recent URL that was being navigated to in the non-AMP window. + * + * @type {?string} + */ + navigateNonAmpUrl; + /** * Non-AMP Link * @@ -83,7 +97,7 @@ class PairedBrowsingApp { /** * Active iframe. * - * @type {HTMLIFrameElement|null} + * @type {?HTMLIFrameElement} */ activeIframe; @@ -104,6 +118,7 @@ class PairedBrowsingApp { // Overlay that is displayed on the client that becomes disconnected. this.disconnectOverlay = document.querySelector( '.disconnect-overlay' ); this.disconnectButtons = { + exit: document.querySelector( '.disconnect-overlay .button.exit' ), goBack: document.querySelector( '.disconnect-overlay .button.go-back' ), }; this.addDisconnectButtonListeners(); @@ -175,6 +190,9 @@ class PairedBrowsingApp { case 'heartbeat': this.receiveHeartbeat( event.source ); break; + case 'navigate': + this.receiveNavigate( event.data, event.source ); + break; default: } } @@ -208,6 +226,21 @@ class PairedBrowsingApp { } } + /** + * Receive navigate. + * + * @param {Object} data Data. + * @param {string} data.href Href. + * @param {Window} sourceWindow The source window. + */ + receiveNavigate( { href }, sourceWindow ) { + if ( isAmpWindow( sourceWindow ) ) { + this.navigateAmpUrl = href; + } else { + this.navigateNonAmpUrl = href; + } + } + /** * Check connected clients. */ @@ -237,6 +270,15 @@ class PairedBrowsingApp { * @param {HTMLIFrameElement} iframe The iframe that hosts the paired browsing client. */ showDisconnectOverlay( iframe ) { + // Show the exit link if we know the URL that the user was last trying to go to. + const navigateUrl = this.ampIframe === iframe ? this.navigateAmpUrl : this.navigateNonAmpUrl; + if ( navigateUrl ) { + this.disconnectButtons.exit.hidden = false; + this.disconnectButtons.exit.href = navigateUrl; + } else { + this.disconnectButtons.exit.hidden = true; + } + // Show the 'Go Back' button if the parent window has history. this.disconnectButtons.goBack.classList.toggle( 'hidden', 0 >= window.history.length ); @@ -344,12 +386,12 @@ class PairedBrowsingApp { /** * Receive loaded. * - * @param {Object} data Data. - * @param {boolean} data.isAmpDocument Whether the document is actually an AMP page. - * @param {string|null} data.ampUrl The AMP URL. - * @param {string|null} data.nonAmpUrl The non-AMP URL. - * @param {string} data.documentTitle The canonical link URL if present. - * @param {Window} sourceWindow The source window. + * @param {Object} data Data. + * @param {boolean} data.isAmpDocument Whether the document is actually an AMP page. + * @param {?string} data.ampUrl The AMP URL. + * @param {?string} data.nonAmpUrl The non-AMP URL. + * @param {string} data.documentTitle The canonical link URL if present. + * @param {Window} sourceWindow The source window. */ receiveLoaded( { isAmpDocument, ampUrl, nonAmpUrl, documentTitle }, sourceWindow ) { const isAmpSource = isAmpWindow( sourceWindow ); diff --git a/assets/src/admin/paired-browsing/client.js b/assets/src/admin/paired-browsing/client.js index 225fb540370..7cf4db27c8e 100644 --- a/assets/src/admin/paired-browsing/client.js +++ b/assets/src/admin/paired-browsing/client.js @@ -100,6 +100,22 @@ function receiveScroll( { x, y } ) { window.scrollTo( x, y ); } +/** + * Handle click event. + * + * @param {MouseEvent} event + */ +function handleClick( event ) { + const element = event.target; + if ( element.matches( '[href]' ) ) { + sendMessage( + parent, + 'navigate', + { href: element.href }, + ); + } +} + /** * Receive replace location. * @@ -140,6 +156,7 @@ if ( isNonAmpWindow( window ) || isAmpWindow( window ) ) { window.addEventListener( 'message', receiveMessage ); window.addEventListener( 'scroll', sendScroll, { passive: true } ); + document.addEventListener( 'click', handleClick, { passive: true } ); sendLoaded(); } ); diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index f6bdaaa0b6e..60125777f84 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -46,9 +46,8 @@
- + +
From 18b32f2ba5586233e4f2d648bd40611ae2bfccef Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Nov 2020 14:34:10 -0800 Subject: [PATCH 015/169] Fix initialization of iframes to populate document title --- assets/src/admin/paired-browsing/app.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index 1e4d03bea72..1f36df2e4e6 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -398,11 +398,6 @@ class PairedBrowsingApp { const sourceIframe = isAmpSource ? this.ampIframe : this.nonAmpIframe; if ( isAmpSource ) { - // Stop if the URL has not changed. - if ( this.currentAmpUrl === ampUrl ) { - return; - } - // Force the AMP iframe to always have an AMP URL. if ( ! isAmpDocument ) { this.replaceLocation( sourceIframe, ampUrl ); @@ -414,11 +409,6 @@ class PairedBrowsingApp { // Update the AMP link above the iframe used for exiting paired browsing. this.ampLink.href = removeQueryArgs( ampUrl, noampQueryVar ); } else { - // Stop if the URL has not changed. - if ( this.currentNonAmpUrl === nonAmpUrl ) { - return; - } - // Force the non-AMP iframe to always have a non-AMP URL. if ( isAmpDocument ) { this.replaceLocation( sourceIframe, nonAmpUrl ); From 9eb39d09761092c9bbd7b14d4b4537c6e4eb6c55 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Nov 2020 17:26:28 -0800 Subject: [PATCH 016/169] Eliminate dependence on window.name --- assets/src/admin/paired-browsing/app.js | 44 +++++++++++++++------- assets/src/admin/paired-browsing/client.js | 37 ++++++++++-------- assets/src/admin/paired-browsing/utils.js | 20 ---------- 3 files changed, 53 insertions(+), 48 deletions(-) delete mode 100644 assets/src/admin/paired-browsing/utils.js diff --git a/assets/src/admin/paired-browsing/app.js b/assets/src/admin/paired-browsing/app.js index 1f36df2e4e6..aa66200f910 100644 --- a/assets/src/admin/paired-browsing/app.js +++ b/assets/src/admin/paired-browsing/app.js @@ -2,11 +2,6 @@ * WordPress dependencies */ import { addQueryArgs, removeQueryArgs } from '@wordpress/url'; -/** - * Internal dependencies - */ -import './app.css'; -import { isNonAmpWindow, isAmpWindow } from './utils'; const { ampPairedBrowsingAppData, history } = window; const { @@ -143,11 +138,31 @@ class PairedBrowsingApp { () => { this.checkConnectedClients(); }, - 500, + 100, ); } ); } + /** + * Return whether the window is for the AMP page. + * + * @param {Window} win Window. + * @return {boolean} Whether AMP window. + */ + isAmpWindow( win ) { + return win === this.ampIframe.contentWindow; + } + + /** + * Return whether the window is for the non-AMP page. + * + * @param {Window} win Window. + * @return {boolean} Whether non-AMP window. + */ + isNonAmpWindow( win ) { + return win === this.nonAmpIframe.contentWindow; + } + /** * Send message to app. * @@ -162,7 +177,7 @@ class PairedBrowsingApp { ...data, ampPairedBrowsing: true, }, - isAmpWindow( win ) ? this.currentAmpUrl : this.currentNonAmpUrl, + this.isAmpWindow( win ) ? this.currentAmpUrl : this.currentNonAmpUrl, ); } @@ -176,7 +191,7 @@ class PairedBrowsingApp { return; } - if ( ! isAmpWindow( event.source ) && ! isNonAmpWindow( event.source ) ) { + if ( ! this.isAmpWindow( event.source ) && ! this.isNonAmpWindow( event.source ) ) { return; } @@ -219,7 +234,7 @@ class PairedBrowsingApp { * @param {Window} sourceWindow The source window. */ receiveHeartbeat( sourceWindow ) { - if ( isAmpWindow( sourceWindow ) ) { + if ( this.isAmpWindow( sourceWindow ) ) { this.ampHeartbeatTimestamp = Date.now(); } else { this.nonAmpHeartbeatTimestamp = Date.now(); @@ -234,7 +249,7 @@ class PairedBrowsingApp { * @param {Window} sourceWindow The source window. */ receiveNavigate( { href }, sourceWindow ) { - if ( isAmpWindow( sourceWindow ) ) { + if ( this.isAmpWindow( sourceWindow ) ) { this.navigateAmpUrl = href; } else { this.navigateNonAmpUrl = href; @@ -245,6 +260,9 @@ class PairedBrowsingApp { * Check connected clients. */ checkConnectedClients() { + this.sendMessage( this.ampIframe.contentWindow, 'init' ); + this.sendMessage( this.nonAmpIframe.contentWindow, 'init' ); + if ( ! this.isClientConnected( this.ampIframe ) ) { this.showDisconnectOverlay( this.ampIframe ); } else if ( ! this.isClientConnected( this.nonAmpIframe ) ) { @@ -367,7 +385,7 @@ class PairedBrowsingApp { receiveScroll( { x, y }, sourceWindow ) { // Rely on scroll event to determine initially-active iframe before mouse first moves. if ( ! this.activeIframe ) { - this.activeIframe = isAmpWindow( sourceWindow ) + this.activeIframe = this.isAmpWindow( sourceWindow ) ? this.ampIframe : this.nonAmpIframe; } @@ -377,7 +395,7 @@ class PairedBrowsingApp { return; } - const otherWindow = isAmpWindow( sourceWindow ) + const otherWindow = this.isAmpWindow( sourceWindow ) ? this.nonAmpIframe.contentWindow : this.ampIframe.contentWindow; this.sendMessage( otherWindow, 'scroll', { x, y } ); @@ -394,7 +412,7 @@ class PairedBrowsingApp { * @param {Window} sourceWindow The source window. */ receiveLoaded( { isAmpDocument, ampUrl, nonAmpUrl, documentTitle }, sourceWindow ) { - const isAmpSource = isAmpWindow( sourceWindow ); + const isAmpSource = this.isAmpWindow( sourceWindow ); const sourceIframe = isAmpSource ? this.ampIframe : this.nonAmpIframe; if ( isAmpSource ) { diff --git a/assets/src/admin/paired-browsing/client.js b/assets/src/admin/paired-browsing/client.js index 7cf4db27c8e..39dd74f1b8f 100644 --- a/assets/src/admin/paired-browsing/client.js +++ b/assets/src/admin/paired-browsing/client.js @@ -3,14 +3,11 @@ */ import domReady from '@wordpress/dom-ready'; -/** - * Internal dependencies - */ -import { isNonAmpWindow, isAmpWindow } from './utils'; - const { parent, ampPairedBrowsingClientData } = window; const { ampUrl, nonAmpUrl, isAmpDocument } = ampPairedBrowsingClientData; +const nonAmpUrlObject = new URL( nonAmpUrl ); + /** * Modify document for paired browsing. */ @@ -51,10 +48,12 @@ function sendMessage( win, type, data = {} ) { ...data, ampPairedBrowsing: true, }, - nonAmpUrl, // Because the paired browsing app is accessed via the canonical URL. + nonAmpUrlObject.origin, // Because the paired browsing app is accessed via the canonical URL. ); } +let initialized = false; + /** * Receive message. * @@ -65,6 +64,12 @@ function receiveMessage( event ) { return; } switch ( event.data.type ) { + case 'init': + if ( ! initialized ) { + initialized = true; + receiveInit( event.data ); + } + break; case 'scroll': receiveScroll( event.data ); break; @@ -148,16 +153,18 @@ function sendHeartbeat() { sendMessage( parent, 'heartbeat' ); } -if ( isNonAmpWindow( window ) || isAmpWindow( window ) ) { +/** + * Receive init. + */ +function receiveInit() { + sendHeartbeat(); setInterval( sendHeartbeat, 500 ); - domReady( () => { - modifyDocumentForPairedBrowsing(); + document.addEventListener( 'click', handleClick, { passive: true } ); + window.addEventListener( 'scroll', sendScroll, { passive: true } ); + domReady( modifyDocumentForPairedBrowsing ); - window.addEventListener( 'message', receiveMessage ); - window.addEventListener( 'scroll', sendScroll, { passive: true } ); - document.addEventListener( 'click', handleClick, { passive: true } ); - - sendLoaded(); - } ); + sendLoaded(); } + +window.addEventListener( 'message', receiveMessage ); diff --git a/assets/src/admin/paired-browsing/utils.js b/assets/src/admin/paired-browsing/utils.js deleted file mode 100644 index 87cba9e8658..00000000000 --- a/assets/src/admin/paired-browsing/utils.js +++ /dev/null @@ -1,20 +0,0 @@ - -/** - * Return whether the window is for the non-AMP page. - * - * @param {Window} win Window. - * @return {boolean} Whether non-AMP window. - */ -export function isNonAmpWindow( win ) { - return win.name === 'paired-browsing-non-amp'; -} - -/** - * Return whether the window is for the AMP page. - * - * @param {Window} win Window. - * @return {boolean} Whether AMP window. - */ -export function isAmpWindow( win ) { - return win.name === 'paired-browsing-amp'; -} From 1711aca77a639db26c48c87e7ade51c4bcd26c92 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Nov 2020 17:33:03 -0800 Subject: [PATCH 017/169] Fix event delegation for navigate messages --- assets/src/admin/paired-browsing/client.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/assets/src/admin/paired-browsing/client.js b/assets/src/admin/paired-browsing/client.js index 39dd74f1b8f..5a2e6e1ab8a 100644 --- a/assets/src/admin/paired-browsing/client.js +++ b/assets/src/admin/paired-browsing/client.js @@ -112,11 +112,12 @@ function receiveScroll( { x, y } ) { */ function handleClick( event ) { const element = event.target; - if ( element.matches( '[href]' ) ) { + const link = element.matches( '[href]' ) ? element : element.closest( '[href]' ); + if ( link ) { sendMessage( parent, 'navigate', - { href: element.href }, + { href: link.href }, ); } } From 9a81427b1c736a499ccc7347a4c8da706a0bc3f6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Nov 2020 17:42:25 -0800 Subject: [PATCH 018/169] Introduce PairedBrowsing service --- .phpstorm.meta.php | 1 + includes/amp-helper-functions.php | 3 - includes/class-amp-theme-support.php | 187 +---------- includes/templates/amp-paired-browsing.php | 3 +- .../class-amp-validated-url-post-type.php | 39 +-- .../class-amp-validation-manager.php | 18 -- src/Admin/PairedBrowsing.php | 301 ++++++++++++++++++ src/AmpWpPlugin.php | 1 + tests/php/test-amp-helper-functions.php | 7 +- tests/php/test-class-amp-theme-support.php | 1 + 10 files changed, 335 insertions(+), 226 deletions(-) create mode 100644 src/Admin/PairedBrowsing.php diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php index 956cef6bc1f..3f7d5073227 100644 --- a/.phpstorm.meta.php +++ b/.phpstorm.meta.php @@ -13,6 +13,7 @@ 'admin.onboarding_wizard' => \AmpProject\AmpWP\Admin\OnboardingWizardSubmenuPage::class, 'admin.options_menu' => \AmpProject\AmpWP\Admin\OptionsMenu::class, 'admin.polyfills' => \AmpProject\AmpWP\Admin\Polyfills::class, + 'admin.paired_browsing' => \AmpProject\AmpWP\Admin\PairedBrowsing::class, 'amp_slug_customization_watcher' => \AmpProject\AmpWP\AmpSlugCustomizationWatcher::class, 'css_transient_cache.ajax_handler' => \AmpProject\AmpWP\Admin\ReenableCssTransientCachingAjaxAction::class, 'css_transient_cache.monitor' => \AmpProject\AmpWP\BackgroundTask\MonitorCssTransientCaching::class, diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index d3b03c08aed..cabde0e6f7f 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1432,9 +1432,6 @@ function amp_get_content_sanitizers( $post = null ) { */ $dev_mode_xpaths = (array) apply_filters( 'amp_dev_mode_element_xpaths', [] ); - // Paired browsing paths. - $dev_mode_xpaths[] = '//script[ contains( text(), "ampPairedBrowsingClient" ) ]'; - if ( is_admin_bar_showing() ) { $dev_mode_xpaths[] = '//*[ @id = "wpadminbar" ]'; $dev_mode_xpaths[] = '//*[ @id = "wpadminbar" ]//*'; diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 3dc9f7a06e1..2db7298d4d1 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -81,13 +81,6 @@ class AMP_Theme_Support { */ const READER_MODE_TEMPLATE_DIRECTORY = 'amp'; - /** - * Query var for requests to open the paired browsing interface. - * - * @var string - */ - const PAIRED_BROWSING_QUERY_VAR = 'amp-paired-browsing'; - /** * Sanitizers, with keys as class names and values as arguments. * @@ -322,12 +315,6 @@ public static function supports_reader_mode() { * @since 0.7 */ public static function finish_init() { - if ( self::is_paired_available() ) { - self::setup_paired_browsing_client(); - add_action( 'template_redirect', [ __CLASS__, 'sanitize_url_for_paired_browsing' ] ); - add_filter( 'template_include', [ __CLASS__, 'serve_paired_browsing_experience' ], PHP_INT_MAX ); - } - if ( ! amp_is_request() ) { /* * Redirect to AMP-less URL if AMP is not available for this URL and yet the query var is present. @@ -459,11 +446,16 @@ public static function redirect_non_amp_url( $status = 302 ) { * When 'amp' theme support has not been added or canonical mode is enabled, then this returns false. * * @since 0.7 + * @deprecated No longer used. Use amp_is_available() and ! amp_is_canonical(). + * @todo There are ecosystem plugins which are still using this method. See . * * @see amp_is_canonical() + * @see amp_is_available() * @return bool Whether available. */ public static function is_paired_available() { + _deprecated_function( __METHOD__, '2.1', '' ); + if ( amp_is_legacy() ) { return false; } @@ -2331,175 +2323,6 @@ public static function enqueue_assets() { wp_enqueue_style( 'amp-default' ); } - /** - * Setup pages to have the paired browsing client script so that the app can interact with it. - * - * @since 1.5.0 - * - * @return void - */ - public static function setup_paired_browsing_client() { - // phpcs:ignore WordPress.Security.NonceVerification.Recommended - if ( isset( $_GET[ self::PAIRED_BROWSING_QUERY_VAR ] ) ) { - return; - } - - // Paired browsing requires a custom script which in turn requires dev mode. - if ( ! amp_is_dev_mode() ) { - return; - } - - /** - * Fires before registering plugin assets that may require core asset polyfills. - * - * @internal - */ - do_action( 'amp_register_polyfills' ); - - $asset_file = AMP__DIR__ . '/assets/js/amp-paired-browsing-client.asset.php'; - $asset = require $asset_file; - $dependencies = $asset['dependencies']; - $version = $asset['version']; - - wp_enqueue_script( - 'amp-paired-browsing-client', - amp_get_asset_url( '/js/amp-paired-browsing-client.js' ), - $dependencies, - $version, - true - ); - - $is_amp_request = amp_is_request(); - $current_url = amp_get_current_url(); - $amp_url = $is_amp_request ? $current_url : amp_add_paired_endpoint( $current_url ); - $non_amp_url = ! $is_amp_request ? $current_url : amp_remove_paired_endpoint( $current_url ); - - wp_localize_script( - 'amp-paired-browsing-client', - 'ampPairedBrowsingClientData', - [ - 'isAmpDocument' => $is_amp_request, - 'ampUrl' => $amp_url, - 'nonAmpUrl' => $non_amp_url, - ] - ); - - // Mark enqueued script for AMP dev mode so that it is not removed. - // @todo Revisit with . - add_filter( - 'script_loader_tag', - static function( $tag, $handle ) { - if ( amp_is_request() && self::has_dependency( wp_scripts(), 'amp-paired-browsing-client', $handle ) ) { - $tag = preg_replace( '/(?<=)/i', ' ' . AMP_Rule_Spec::DEV_MODE_ATTRIBUTE, $tag ); - } - return $tag; - }, - 10, - 2 - ); - } - - /** - * Get paired browsing URL for a given URL. - * - * @since 1.5.0 - * - * @param string $url URL. - * @return string Paired browsing URL. - */ - public static function get_paired_browsing_url( $url = null ) { - if ( ! $url ) { - $url = amp_get_current_url(); - } - $url = amp_remove_paired_endpoint( $url ); - $url = remove_query_arg( - [ QueryVar::NOAMP, AMP_Validated_URL_Post_Type::VALIDATE_ACTION, AMP_Validation_Manager::VALIDATION_ERROR_TERM_STATUS_QUERY_VAR ], - $url - ); - $url = add_query_arg( self::PAIRED_BROWSING_QUERY_VAR, '1', $url ); - return $url; - } - - /** - * Remove any unnecessary query vars that could hamper the paired browsing experience. - * - * @since 1.5.0 - */ - public static function sanitize_url_for_paired_browsing() { - if ( isset( $_GET[ self::PAIRED_BROWSING_QUERY_VAR ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended - $original_url = wp_unslash( $_SERVER['REQUEST_URI'] ); - $updated_url = self::get_paired_browsing_url( $original_url ); - if ( $updated_url !== $original_url ) { - wp_safe_redirect( $updated_url ); - exit; - } - } - } - - /** - * Serve paired browsing experience if it is being requested. - * - * Includes a custom template that acts as an interface to facilitate a side-by-side comparison of a - * non-AMP page and its AMP version to review any discrepancies. - * - * @since 1.5.0 - * - * @param string $template Path of the template to include. - * @return string Custom template if in paired browsing mode, else the supplied template. - */ - public static function serve_paired_browsing_experience( $template ) { - // phpcs:ignore WordPress.Security.NonceVerification.Recommended - if ( ! isset( $_GET[ self::PAIRED_BROWSING_QUERY_VAR ] ) ) { - return $template; - } - - if ( ! amp_is_dev_mode() ) { - wp_die( - esc_html__( 'Paired browsing is only available when AMP dev mode is enabled (e.g. when logged-in and admin bar is showing).', 'amp' ), - esc_html__( 'AMP Paired Browsing Unavailable', 'amp' ), - [ 'response' => 403 ] - ); - } - - /** This action is documented in includes/class-amp-theme-support.php */ - do_action( 'amp_register_polyfills' ); - - wp_enqueue_style( - 'amp-paired-browsing-app', - amp_get_asset_url( '/css/amp-paired-browsing-app.css' ), - [ 'dashicons' ], - AMP__VERSION - ); - - wp_styles()->add_data( 'amp-paired-browsing-app', 'rtl', 'replace' ); - - $asset_file = AMP__DIR__ . '/assets/js/amp-paired-browsing-app.asset.php'; - $asset = require $asset_file; - $dependencies = $asset['dependencies']; - $version = $asset['version']; - - wp_enqueue_script( - 'amp-paired-browsing-app', - amp_get_asset_url( '/js/amp-paired-browsing-app.js' ), - $dependencies, - $version, - true - ); - - wp_localize_script( - 'amp-paired-browsing-app', - 'ampPairedBrowsingAppData', - [ - 'ampPairedBrowsingQueryVar' => self::PAIRED_BROWSING_QUERY_VAR, - 'noampQueryVar' => QueryVar::NOAMP, - 'noampMobile' => QueryVar::NOAMP_MOBILE, - 'documentTitlePrefix' => __( 'AMP Paired Browsing:', 'amp' ), - ] - ); - - return AMP__DIR__ . '/includes/templates/amp-paired-browsing.php'; - } - /** * Print the important emoji-related styles. * diff --git a/includes/templates/amp-paired-browsing.php b/includes/templates/amp-paired-browsing.php index 60125777f84..a2da4fb6fb9 100644 --- a/includes/templates/amp-paired-browsing.php +++ b/includes/templates/amp-paired-browsing.php @@ -6,8 +6,9 @@ */ use AmpProject\AmpWP\QueryVar; +use AmpProject\AmpWP\Admin\PairedBrowsing; -$url = remove_query_arg( [ AMP_Theme_Support::PAIRED_BROWSING_QUERY_VAR, QueryVar::NOAMP ], amp_get_current_url() ); +$url = remove_query_arg( [ PairedBrowsing::APP_QUERY_VAR, QueryVar::NOAMP ], amp_get_current_url() ); $non_amp_url = add_query_arg( QueryVar::NOAMP, QueryVar::NOAMP_MOBILE, $url ); $amp_url = amp_add_paired_endpoint( $url ); ?> diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php index 95d934d1b5a..26c456daf1e 100644 --- a/includes/validation/class-amp-validated-url-post-type.php +++ b/includes/validation/class-amp-validated-url-post-type.php @@ -2199,41 +2199,42 @@ public static function print_status_meta_box( $post ) {
ID, self::QUERIED_OBJECT_POST_META_KEY, true ); if ( isset( $queried_object['id'], $queried_object['type'] ) ) { - $after = ' | '; if ( 'post' === $queried_object['type'] && get_post( $queried_object['id'] ) && post_type_exists( get_post( $queried_object['id'] )->post_type ) ) { $post_type_object = get_post_type_object( get_post( $queried_object['id'] )->post_type ); - edit_post_link( $post_type_object->labels->edit_item, '', $after, $queried_object['id'] ); + edit_post_link( $post_type_object->labels->edit_item, '', '', $queried_object['id'] ); $view_label = $post_type_object->labels->view_item; } elseif ( 'term' === $queried_object['type'] && get_term( $queried_object['id'] ) && taxonomy_exists( get_term( $queried_object['id'] )->taxonomy ) ) { $taxonomy_object = get_taxonomy( get_term( $queried_object['id'] )->taxonomy ); - edit_term_link( $taxonomy_object->labels->edit_item, '', $after, get_term( $queried_object['id'] ) ); + edit_term_link( $taxonomy_object->labels->edit_item, '', '', get_term( $queried_object['id'] ) ); $view_label = $taxonomy_object->labels->view_item; } elseif ( 'user' === $queried_object['type'] ) { $link = get_edit_user_link( $queried_object['id'] ); if ( $link ) { - printf( '%s%s', esc_url( $link ), esc_html__( 'Edit User', 'amp' ), esc_html( $after ) ); + printf( '%s', esc_url( $link ), esc_html__( 'Edit User', 'amp' ) ); } $view_label = __( 'View User', 'amp' ); } } - printf( '%s', esc_url( self::get_url_from_post( $post ) ), esc_html( $view_label ) ); - - if ( - $is_amp_enabled - && - AMP_Theme_Support::TRANSITIONAL_MODE_SLUG === AMP_Options_Manager::get_option( Option::THEME_SUPPORT ) - && - AMP_Theme_Support::is_paired_available() - ) { - printf( - ' | %s', - esc_url( AMP_Theme_Support::get_paired_browsing_url( self::get_url_from_post( $post ) ) ), - esc_html__( 'Paired Browsing', 'amp' ) - ); - } + $actions['edit'] = ob_get_clean(); + $actions['view'] = sprintf( '%s', esc_url( self::get_url_from_post( $post ) ), esc_html( $view_label ) ); + + /** + * Filters the array of action links shown in the status metabox. + * + * @since 2.1 + * @internal + * + * @param string[] $actions Action links. + */ + $actions = apply_filters( 'amp_validated_url_status_actions', $actions, $post ); + + echo implode( ' | ', array_filter( $actions ) ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?>
diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 6f0cde16897..93f0bbc0319 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -413,24 +413,6 @@ public static function add_admin_bar_menu_items( $wp_admin_bar ) { $wp_admin_bar->add_node( $validate_item ); } - if ( - AMP_Theme_Support::TRANSITIONAL_MODE_SLUG === AMP_Options_Manager::get_option( Option::THEME_SUPPORT ) - && - AMP_Theme_Support::is_paired_available() - && - amp_is_dev_mode() - ) { - // Construct admin bar item to link to paired browsing experience. - $paired_browsing_item = [ - 'parent' => 'amp', - 'id' => 'amp-paired-browsing', - 'title' => esc_html__( 'Paired Browsing', 'amp' ), - 'href' => AMP_Theme_Support::get_paired_browsing_url(), - ]; - - $wp_admin_bar->add_node( $paired_browsing_item ); - } - // Add settings link to admin bar. if ( current_user_can( 'manage_options' ) ) { $wp_admin_bar->add_node( diff --git a/src/Admin/PairedBrowsing.php b/src/Admin/PairedBrowsing.php new file mode 100644 index 00000000000..d7a6511b4ab --- /dev/null +++ b/src/Admin/PairedBrowsing.php @@ -0,0 +1,301 @@ +dev_tools_user_access = $dev_tools_user_access; + } + + /** + * Adds the filters. + */ + public function register() { + add_action( 'wp', [ $this, 'init_frontend' ], PHP_INT_MAX ); + add_filter( 'amp_dev_mode_element_xpaths', [ $this, 'filter_dev_mode_element_xpaths' ] ); + add_filter( 'amp_validated_url_status_actions', [ $this, 'filter_validated_url_status_actions' ], 10, 2 ); + } + + /** + * Filter Dev Mode XPaths to include the inline script used by the client. + * + * @param string[] $xpaths Element XPaths. + * @return string[] XPaths. + */ + public function filter_dev_mode_element_xpaths( $xpaths ) { + $xpaths[] = '//script[ @id = "amp-paired-browsing-client-js-before" ]'; + return $xpaths; + } + + /** + * Filter the status actions for a validated URL to add the paired browsing link. + * + * @param string[] $actions Action links. + * @param WP_Post $post AMP Validated URL post. + * @return string[] Actions. + */ + public function filter_validated_url_status_actions( $actions, WP_Post $post ) { + $actions['paired_browsing'] = sprintf( + '%s', + esc_url( $this->get_paired_browsing_url( AMP_Validated_URL_Post_Type::get_url_from_post( $post ) ) ), + esc_html__( 'Paired Browsing', 'amp' ) + ); + return $actions; + } + + /** + * Initialize frontend. + */ + public function init_frontend() { + if ( ! amp_is_available() || ! amp_is_dev_mode() ) { + return; + } + + if ( isset( $_GET[ self::APP_QUERY_VAR ] ) ) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended + $this->init_app(); + } else { + $this->init_client(); + } + } + + /** + * Set up app. + * + * This is the parent request that has the iframes for both AMP and non-AMP. + */ + public function init_app() { + add_action( 'template_redirect', [ $this, 'ensure_app_location' ] ); + add_action( 'template_include', [ $this, 'filter_template_include_for_app' ], PHP_INT_MAX ); + } + + /** + * Set up client. + * + * Make sure pages have the paired browsing client script so that the app can interact with it. + */ + public function init_client() { + add_action( 'admin_bar_menu', [ $this, 'add_admin_bar_menu_item' ], 102 ); + + /** + * Fires before registering plugin assets that may require core asset polyfills. + * + * @internal + */ + do_action( 'amp_register_polyfills' ); + + $handle = 'amp-paired-browsing-client'; + $asset = require AMP__DIR__ . '/assets/js/amp-paired-browsing-client.asset.php'; + $dependencies = $asset['dependencies']; + $version = $asset['version']; + + wp_enqueue_script( + $handle, + amp_get_asset_url( '/js/amp-paired-browsing-client.js' ), + $dependencies, + $version, + true + ); + + $is_amp_request = amp_is_request(); + $current_url = amp_get_current_url(); + $amp_url = $is_amp_request ? $current_url : amp_add_paired_endpoint( $current_url ); + $non_amp_url = ! $is_amp_request ? $current_url : amp_remove_paired_endpoint( $current_url ); + + wp_add_inline_script( + $handle, + sprintf( + 'var ampPairedBrowsingClientData = %s;', + wp_json_encode( + [ + 'isAmpDocument' => $is_amp_request, + 'ampUrl' => $amp_url, + 'nonAmpUrl' => $non_amp_url, + ] + ) + ), + 'before' + ); + + // Mark enqueued script for AMP dev mode so that it is not removed. + // @todo Revisit with . + $dev_mode_handles = array_merge( + [ $handle, 'wp-i18n' ], + $dependencies + ); + add_filter( + 'script_loader_tag', + static function( $tag, $script_handle ) use ( $dev_mode_handles ) { + if ( amp_is_request() && in_array( $script_handle, $dev_mode_handles, true ) ) { + $tag = preg_replace( '/(?<=)/i', ' ' . DevMode::DEV_MODE_ATTRIBUTE, $tag ); + } + return $tag; + }, + 10, + 2 + ); + } + + /** + * Add paired browsing menu item to admin bar for AMP. + * + * @param WP_Admin_Bar $wp_admin_bar Admin bar. + */ + public function add_admin_bar_menu_item( WP_Admin_Bar $wp_admin_bar ) { + if ( $this->dev_tools_user_access->is_user_enabled() ) { + $wp_admin_bar->add_node( + [ + 'parent' => 'amp', + 'id' => 'amp-paired-browsing', + 'title' => esc_html__( 'Paired Browsing', 'amp' ), + 'href' => $this->get_paired_browsing_url(), + ] + ); + } + } + + /** + * Get paired browsing URL for a given URL. + * + * @param string $url URL. + * @return string Paired browsing URL. + */ + public function get_paired_browsing_url( $url = null ) { + if ( ! $url ) { + $url = amp_get_current_url(); + } + $url = amp_remove_paired_endpoint( $url ); + $url = remove_query_arg( + [ QueryVar::NOAMP, AMP_Validated_URL_Post_Type::VALIDATE_ACTION, AMP_Validation_Manager::VALIDATION_ERROR_TERM_STATUS_QUERY_VAR ], + $url + ); + $url = add_query_arg( self::APP_QUERY_VAR, '1', $url ); + return $url; + } + + /** + * Remove any unnecessary query vars that could hamper the paired browsing experience. + * + * @return bool Whether redirection was done. + */ + public function ensure_app_location() { + $original_url = amp_get_current_url(); + $updated_url = $this->get_paired_browsing_url( $original_url ); + if ( $updated_url === $original_url ) { + return false; + } + + if ( wp_safe_redirect( $updated_url ) ) { + exit; + } + return true; + } + + /** + * Serve paired browsing experience if it is being requested. + * + * Includes a custom template that acts as an interface to facilitate a side-by-side comparison of a + * non-AMP page and its AMP version to review any discrepancies. + * + * @return string Custom template if in paired browsing mode, else the supplied template. + */ + public function filter_template_include_for_app() { + if ( ! amp_is_dev_mode() ) { + wp_die( + esc_html__( 'Paired browsing is only available when AMP dev mode is enabled (e.g. when logged-in and admin bar is showing).', 'amp' ), + esc_html__( 'AMP Paired Browsing Unavailable', 'amp' ), + [ 'response' => 403 ] + ); + } + + /** This action is documented in includes/class-amp-theme-support.php */ + do_action( 'amp_register_polyfills' ); + + wp_enqueue_style( + 'amp-paired-browsing-app', + amp_get_asset_url( '/css/amp-paired-browsing-app.css' ), + [ 'dashicons' ], + AMP__VERSION + ); + + wp_styles()->add_data( 'amp-paired-browsing-app', 'rtl', 'replace' ); + + $asset = require AMP__DIR__ . '/assets/js/amp-paired-browsing-app.asset.php'; + $dependencies = $asset['dependencies']; + $version = $asset['version']; + + wp_enqueue_script( + 'amp-paired-browsing-app', + amp_get_asset_url( '/js/amp-paired-browsing-app.js' ), + $dependencies, + $version, + true + ); + + wp_localize_script( + 'amp-paired-browsing-app', + 'ampPairedBrowsingAppData', + [ + 'ampPairedBrowsingQueryVar' => self::APP_QUERY_VAR, + 'noampQueryVar' => QueryVar::NOAMP, + 'noampMobile' => QueryVar::NOAMP_MOBILE, + 'documentTitlePrefix' => __( 'AMP Paired Browsing:', 'amp' ), + ] + ); + + return AMP__DIR__ . '/includes/templates/amp-paired-browsing.php'; + } +} diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php index 6275b6e70a6..48e38e0d194 100644 --- a/src/AmpWpPlugin.php +++ b/src/AmpWpPlugin.php @@ -61,6 +61,7 @@ final class AmpWpPlugin extends ServiceBasedPlugin { 'admin.onboarding_wizard' => Admin\OnboardingWizardSubmenuPage::class, 'admin.options_menu' => Admin\OptionsMenu::class, 'admin.polyfills' => Admin\Polyfills::class, + 'admin.paired_amp_routing' => Admin\PairedBrowsing::class, 'amp_slug_customization_watcher' => AmpSlugCustomizationWatcher::class, 'css_transient_cache.ajax_handler' => Admin\ReenableCssTransientCachingAjaxAction::class, 'css_transient_cache.monitor' => BackgroundTask\MonitorCssTransientCaching::class, diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 0c610a7dbbd..d1ea72f490c 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -809,12 +809,13 @@ public function test_amp_add_amphtml_link_transitional_mode( $data_provider ) { $assert_amphtml_link_present = function() use ( $amphtml_url, $get_amp_html_link, $available ) { if ( $available ) { - $this->assertTrue( AMP_Theme_Support::is_paired_available() ); + $this->assertTrue( amp_is_available() ); $this->assertEquals( sprintf( '', esc_url( $amphtml_url ) ), $get_amp_html_link() ); } else { + $this->assertFalse( amp_is_available() ); $this->assertStringStartsWith( '