diff --git a/amp.php b/amp.php index f1534a491e0..0daee30a4b0 100644 --- a/amp.php +++ b/amp.php @@ -312,7 +312,7 @@ function amp_load_classes() { * @since 0.2 */ function amp_add_frontend_actions() { - require_once AMP__DIR__ . '/includes/amp-frontend-actions.php'; + add_action( 'wp_head', 'amp_add_amphtml_link' ); } /** @@ -436,8 +436,12 @@ function _amp_bootstrap_customizer() { */ function amp_redirect_old_slug_to_new_url( $link ) { - if ( is_amp_endpoint() ) { - $link = trailingslashit( trailingslashit( $link ) . amp_get_slug() ); + if ( is_amp_endpoint() && ! amp_is_canonical() ) { + if ( current_theme_supports( 'amp' ) ) { + $link = add_query_arg( amp_get_slug(), '', $link ); + } else { + $link = trailingslashit( trailingslashit( $link ) . amp_get_slug() ); + } } return $link; diff --git a/includes/amp-frontend-actions.php b/includes/amp-frontend-actions.php index f5d7ebca13d..75ce9030a35 100644 --- a/includes/amp-frontend-actions.php +++ b/includes/amp-frontend-actions.php @@ -2,39 +2,21 @@ /** * Callbacks for adding AMP-related things to the main theme. * + * @deprecated Function in this file has been moved to amp-helper-functions.php. * @package AMP */ -add_action( 'wp_head', 'amp_frontend_add_canonical' ); +_deprecated_file( __FILE__, '1.0', null, esc_html__( 'Use amp_add_amphtml_link() function which is already included from amp-helper-functions.php', 'amp' ) ); /** * Add amphtml link to frontend. * - * @todo This function's name is incorrect. It's not about adding a canonical link but adding the amphtml link. + * @deprecated * * @since 0.2 + * @since 1.0 Deprecated */ function amp_frontend_add_canonical() { - - /** - * Filters whether to show the amphtml link on the frontend. - * - * @todo This filter's name is incorrect. It's not about adding a canonical link but adding the amphtml link. - * @since 0.2 - */ - if ( false === apply_filters( 'amp_frontend_show_canonical', true ) ) { - return; - } - - $amp_url = null; - if ( is_singular() ) { - $amp_url = amp_get_permalink( get_queried_object_id() ); - } elseif ( isset( $_SERVER['REQUEST_URI'] ) ) { - $host_url = preg_replace( '#(^https?://[^/]+)/.*#', '$1', home_url( '/' ) ); - $self_url = esc_url_raw( $host_url . wp_unslash( $_SERVER['REQUEST_URI'] ) ); - $amp_url = add_query_arg( amp_get_slug(), '', $self_url ); - } - if ( $amp_url ) { - printf( '', esc_url( $amp_url ) ); - } + _deprecated_function( __FUNCTION__, '1.0', 'amp_add_amphtml_link' ); + amp_add_amphtml_link(); } diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 35e5b16cc1e..fdb513b6eb0 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -49,6 +49,26 @@ function amp_get_slug() { return $query_var; } +/** + * Get the URL for the current request. + * + * This is essentially the REQUEST_URI prefixed by the scheme and host for the home URL. + * This is needed in particular due to subdirectory installs. + * + * @since 1.0 + * + * @return string Current URL. + */ +function amp_get_current_url() { + $url = preg_replace( '#(^https?://[^/]+)/.*#', '$1', home_url( '/' ) ); + if ( isset( $_SERVER['REQUEST_URI'] ) ) { + $url = esc_url_raw( $url . wp_unslash( $_SERVER['REQUEST_URI'] ) ); + } else { + $url .= '/'; + } + return $url; +} + /** * Retrieves the full AMP-specific permalink for the given post ID. * @@ -146,6 +166,34 @@ function amp_remove_endpoint( $url ) { return $url; } +/** + * Add amphtml link. + * + * @since 1.0 + */ +function amp_add_amphtml_link() { + + /** + * Filters whether to show the amphtml link on the frontend. + * + * @todo This filter's name is incorrect. It's not about adding a canonical link but adding the amphtml link. + * @since 0.2 + */ + if ( false === apply_filters( 'amp_frontend_show_canonical', true ) ) { + return; + } + + $amp_url = null; + if ( is_singular() ) { + $amp_url = amp_get_permalink( get_queried_object_id() ); + } else { + $amp_url = add_query_arg( amp_get_slug(), '', amp_get_current_url() ); + } + if ( $amp_url ) { + printf( '', esc_url( $amp_url ) ); + } +} + /** * Determine whether a given post supports AMP. * diff --git a/includes/amp-post-template-actions.php b/includes/amp-post-template-actions.php index 88d717c2c6e..fa3fc301a28 100644 --- a/includes/amp-post-template-actions.php +++ b/includes/amp-post-template-actions.php @@ -2,6 +2,7 @@ /** * Callbacks for adding content to an AMP template. * + * @todo Rename this file from amp-post-template-actions.php to amp-post-template-functions.php. * @package AMP */ diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 5097844e350..9c1ee245c94 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -139,19 +139,15 @@ public static function init() { */ public static function finish_init() { if ( ! is_amp_endpoint() ) { - // Add amphtml link when paired mode is available. if ( self::is_paired_available() ) { - amp_add_frontend_actions(); // @todo This function is poor in how it requires a file that then does add_action(). - if ( ! has_action( 'wp_head', 'amp_frontend_add_canonical' ) ) { - add_action( 'wp_head', 'amp_frontend_add_canonical' ); - } + amp_add_frontend_actions(); } return; } - if ( amp_is_canonical() ) { - self::redirect_canonical_amp(); - } else { + self::ensure_proper_amp_location(); + + if ( ! amp_is_canonical() ) { self::register_paired_hooks(); } @@ -168,32 +164,75 @@ public static function finish_init() { } /** - * Redirect to canonical URL if the AMP URL was loaded, since canonical is now AMP. + * Ensure that the current AMP location is correct. * - * @since 0.7 - * @since 1.0 Added $exit param. - * @todo Rename to redirect_non_amp(). + * @since 1.0 * * @param bool $exit Whether to exit after redirecting. + * @return bool Whether redirection was done. Naturally this is irrelevant if $exit is true. */ - public static function redirect_canonical_amp( $exit = true ) { - if ( false !== get_query_var( amp_get_slug(), false ) ) { // Because is_amp_endpoint() now returns true if amp_is_canonical(). - $url = preg_replace( '#^(https?://.+?)(/.*)$#', '$1', home_url( '/' ) ); - if ( isset( $_SERVER['REQUEST_URI'] ) ) { - $url .= wp_unslash( $_SERVER['REQUEST_URI'] ); - } - - $url = amp_remove_endpoint( $url ); + public static function ensure_proper_amp_location( $exit = true ) { + $has_query_var = false !== get_query_var( amp_get_slug(), false ); // May come from URL param or endpoint slug. + $has_url_param = isset( $_GET[ amp_get_slug() ] ); // WPCS: CSRF OK. + if ( amp_is_canonical() ) { + /* + * When AMP native/canonical, then when there is an /amp/ endpoint or ?amp URL param, + * then a redirect needs to be done to the URL without any AMP indicator in the URL. + */ + if ( $has_query_var || $has_url_param ) { + return self::redirect_ampless_url( $exit ); + } + } else { /* - * Temporary redirect because AMP URL may return when blocking validation errors - * occur or when a non-canonical AMP theme is used. + * When in AMP paired mode *with* theme support, then the proper AMP URL has the 'amp' URL param + * and not the /amp/ endpoint. The URL param is now the exclusive way to mark AMP in paired mode + * when amp theme support present. This is important for plugins to be able to reliably call + * is_amp_endpoint() before the parse_query action. */ - wp_safe_redirect( $url, 302 ); - if ( $exit ) { - exit; + if ( $has_query_var && ! $has_url_param ) { + $old_url = amp_get_current_url(); + $new_url = add_query_arg( amp_get_slug(), '', amp_remove_endpoint( $old_url ) ); + if ( $old_url !== $new_url ) { + wp_safe_redirect( $new_url, 302 ); + if ( $exit ) { + exit; + } + return true; + } } } + return false; + } + + /** + * Redirect to non-AMP version of the current URL, such as because AMP is canonical or there are unaccepted validation errors. + * + * If the current URL is already AMP-less then do nothing. + * + * @since 0.7 + * @since 1.0 Added $exit param. + * @since 1.0 Renamed from redirect_canonical_amp(). + * + * @param bool $exit Whether to exit after redirecting. + * @return bool Whether redirection was done. Naturally this is irrelevant if $exit is true. + */ + public static function redirect_ampless_url( $exit = true ) { + $current_url = amp_get_current_url(); + $ampless_url = amp_remove_endpoint( $current_url ); + if ( $ampless_url === $current_url ) { + return false; + } + + /* + * Temporary redirect because AMP URL may return when blocking validation errors + * occur or when a non-canonical AMP theme is used. + */ + wp_safe_redirect( $ampless_url, 302 ); + if ( $exit ) { + exit; + } + return true; } /** @@ -1162,7 +1201,7 @@ public static function prepare_response( $response, $args = array() ) { $head->appendChild( $script ); } } else { - self::redirect_canonical_amp( false ); + self::redirect_ampless_url( false ); return esc_html__( 'Redirecting to non-AMP version.', 'amp' ); } } diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index aa9e9b71f1d..f420eb5da5a 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -137,6 +137,7 @@ public static function add_admin_hooks() { $query_vars[] = 'amp_taxonomy_terms_updated'; $query_vars[] = self::REMAINING_ERRORS; $query_vars[] = 'amp_urls_tested'; + $query_vars[] = 'amp_validate_error'; return $query_vars; } ); } @@ -277,24 +278,52 @@ public static function display_invalid_url_validation_error_counts_summary( $pos * @return WP_Post|null The post of the existing custom post, or null. */ public static function get_invalid_url_post( $url ) { + $url = remove_query_arg( amp_get_slug(), $url ); return get_page_by_path( md5( $url ), OBJECT, self::POST_TYPE_SLUG ); } + /** + * Get the URL from a given amp_invalid_url post. + * + * The URL will be returned with the amp query var added to it if the site is not canonical. The post_title + * is always stored using the canonical AMP-less URL. + * + * @param int|WP_post $post Post. + * @return string|null The URL stored for the post or null if post does not exist or it is not the right type. + */ + public static function get_url_from_post( $post ) { + $post = get_post( $post ); + if ( ! $post || self::POST_TYPE_SLUG !== $post->post_type ) { + return null; + } + $url = $post->post_title; + if ( ! amp_is_canonical() ) { + $url = add_query_arg( amp_get_slug(), '', $url ); + } + return $url; + } + /** * Stores the validation errors. * * If there are no validation errors provided, then any existing amp_invalid_url post is deleted. * - * @param array $validation_errors Validation errors. - * @param string $url URL on which the validation errors occurred. + * @param array $validation_errors Validation errors. + * @param string $url URL on which the validation errors occurred. Will be normalized to non-AMP version. + * @param int|WP_Post $post Post to update. Optional. If empty, then post is looked up by URL. * @return int|WP_Error $post_id The post ID of the custom post type used, null if post was deleted due to no validation errors, or WP_Error on failure. * @global WP $wp */ - public static function store_validation_errors( $validation_errors, $url ) { - $post_slug = md5( $url ); - $post = get_page_by_path( $post_slug, OBJECT, self::POST_TYPE_SLUG ); - if ( ! $post ) { - $post = get_page_by_path( $post_slug . '__trashed', OBJECT, self::POST_TYPE_SLUG ); + public static function store_validation_errors( $validation_errors, $url, $post = null ) { + $url = remove_query_arg( amp_get_slug(), $url ); // Only ever store the canonical version. + $slug = md5( $url ); + if ( $post ) { + $post = get_post( $post ); + } else { + $post = get_page_by_path( $slug, OBJECT, self::POST_TYPE_SLUG ); + if ( ! $post ) { + $post = get_page_by_path( $slug . '__trashed', OBJECT, self::POST_TYPE_SLUG ); + } } // Since there are no validation errors and there is an existing $existing_post_id, just delete the post. @@ -367,7 +396,7 @@ public static function store_validation_errors( $validation_errors, $url ) { 'ID' => $post ? $post->ID : null, 'post_type' => self::POST_TYPE_SLUG, 'post_title' => $url, - 'post_name' => $post_slug, + 'post_name' => $slug, 'post_content' => $placeholder, // Content is provided via wp_insert_post_data filter above to guard against Kses-corruption. 'post_status' => 'publish', ) ), @@ -609,10 +638,15 @@ public static function filter_row_actions( $actions, $post ) { esc_html__( 'Details', 'amp' ) ); unset( $actions['inline hide-if-no-js'] ); - $url = $post->post_title; - $view_url = add_query_arg( AMP_Validation_Manager::VALIDATE_QUERY_VAR, '', $url ); // Prevent redirection to non-AMP page. - $actions['view'] = sprintf( '%s', esc_url( $view_url ), esc_html__( 'View', 'amp' ) ); + $url = self::get_url_from_post( $post ); + if ( $url ) { + $actions['view'] = sprintf( + '%s', + esc_url( add_query_arg( AMP_Validation_Manager::VALIDATE_QUERY_VAR, '', $url ) ), + esc_html__( 'View', 'amp' ) + ); + } $actions[ self::RECHECK_ACTION ] = sprintf( '%s', @@ -648,33 +682,43 @@ public static function handle_bulk_action( $redirect, $action, $items ) { return $redirect; } $remaining_invalid_urls = array(); + + $errors = array(); + foreach ( $items as $item ) { $post = get_post( $item ); if ( empty( $post ) ) { continue; } - $url = $post->post_title; + + $url = self::get_url_from_post( $post ); if ( empty( $url ) ) { continue; } - $validation_errors = AMP_Validation_Manager::validate_url( $url ); - if ( ! is_array( $validation_errors ) ) { + $validity = AMP_Validation_Manager::validate_url( $url ); + if ( is_wp_error( $validity ) ) { + $errors[] = $validity->get_error_code(); continue; } - self::store_validation_errors( $validation_errors, $url ); - if ( ! empty( $validation_errors ) ) { - $remaining_invalid_urls[] = $url; + self::store_validation_errors( $validity['validation_errors'], $validity['url'], $post ); + if ( ! empty( $validity['validation_errors'] ) ) { + $remaining_invalid_urls[] = $validity['url']; } } // Get the URLs that still have errors after rechecking. $args = array( - self::URLS_TESTED => count( $items ), - self::REMAINING_ERRORS => empty( $remaining_invalid_urls ) ? '0' : '1', + self::URLS_TESTED => count( $items ), ); + if ( ! empty( $errors ) ) { + $args['amp_validate_error'] = $errors; + } else { + $args[ self::REMAINING_ERRORS ] = count( $remaining_invalid_urls ); + } + $redirect = remove_query_arg( wp_removable_query_args(), $redirect ); return add_query_arg( $args, $redirect ); } @@ -688,6 +732,31 @@ public static function print_admin_notice() { return; } + if ( isset( $_GET['amp_validate_error'] ) ) { // WPCS: CSRF OK. + $error_codes = array_unique( array_map( 'sanitize_key', (array) $_GET['amp_validate_error'] ) ); // WPCS: CSRF OK. + foreach ( $error_codes as $error_code ) { + switch ( $error_code ) { + case 'http_request_failed': + $message = __( 'Failed to fetch URL(s) to validate. This may be due to a request timeout.', 'amp' ); + break; + case '404': + $message = __( 'The fetched URL(s) was not found. It may have been deleted. If so, you can trash this.', 'amp' ); + break; + case '500': + $message = __( 'An internal server error occurred when fetching the URL.', 'amp' ); + break; + default: + /* translators: %s is error code */ + $message = sprintf( __( 'Unable to validate the URL(s); error code is %s.', 'amp' ), $error_code ); // Note that $error_code has been sanitized with sanitize_key(); will be escaped below as well. + } + printf( + '

%s

', + esc_html( $message ), + esc_html__( 'Dismiss this notice.', 'amp' ) + ); + } + } + if ( isset( $_GET[ self::REMAINING_ERRORS ] ) ) { $count_urls_tested = isset( $_GET[ self::URLS_TESTED ] ) ? intval( $_GET[ self::URLS_TESTED ] ) : 1; // WPCS: CSRF ok. $errors_remain = ! empty( $_GET[ self::REMAINING_ERRORS ] ); // WPCS: CSRF ok. @@ -743,7 +812,7 @@ public static function handle_inline_recheck( $post_id ) { $redirect = remove_query_arg( wp_removable_query_args(), $redirect ); } - if ( ! $redirect || is_wp_error( $validation_results ) || empty( $validation_results ) ) { + if ( ! $redirect || empty( $validation_results ) ) { // If there are no remaining errors and the post was deleted, redirect to edit.php instead of post.php. $redirect = add_query_arg( 'post_type', @@ -755,8 +824,9 @@ public static function handle_inline_recheck( $post_id ) { self::URLS_TESTED => '1', ); - // @todo For WP_Error case, see . - if ( ! is_wp_error( $validation_results ) ) { + if ( is_wp_error( $validation_results ) ) { + $args['amp_validate_error'] = $validation_results->get_error_code(); + } else { $args[ self::REMAINING_ERRORS ] = count( array_filter( $validation_results, function( $result ) { @@ -777,16 +847,19 @@ function( $result ) { */ public static function recheck_post( $post ) { $post = get_post( $post ); - $url = $post->post_title; + $url = self::get_url_from_post( $post ); + if ( ! $url ) { + return new WP_Error( 'missing_url' ); + } - $validation_errors = AMP_Validation_Manager::validate_url( $url ); - if ( is_wp_error( $validation_errors ) ) { - return $validation_errors; + $validity = AMP_Validation_Manager::validate_url( $url ); + if ( is_wp_error( $validity ) ) { + return $validity; } $validation_results = array(); - self::store_validation_errors( $validation_errors, $url ); - foreach ( $validation_errors as $error ) { + self::store_validation_errors( $validity['validation_errors'], $validity['url'], $post->ID ); + foreach ( $validity['validation_errors'] as $error ) { $sanitized = AMP_Validation_Error_Taxonomy::is_validation_error_sanitized( $error ); $validation_results[] = compact( 'error', 'sanitized' ); @@ -963,6 +1036,8 @@ public static function print_status_meta_box( $post ) { public static function print_validation_errors_meta_box( $post ) { $validation_errors = self::get_invalid_url_validation_errors( $post ); + $url = self::get_url_from_post( $post ); + $can_serve_amp = 0 === count( array_filter( $validation_errors, function( $validation_error ) { return AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS !== $validation_error['status']; } ) ); @@ -994,7 +1069,7 @@ public static function print_validation_errors_meta_box( $post ) {