From 8643b8b9d4a9036cfffb25ab1055b464c0da92a8 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 13 Apr 2018 16:54:29 -0700 Subject: [PATCH 01/27] Add initial AMP_Core_Theme_Sanitizer with partial Twenty Seventeen fixups --- includes/class-amp-autoloader.php | 1 + includes/class-amp-theme-support.php | 8 ++ .../class-amp-core-theme-sanitizer.php | 91 +++++++++++++++++++ 3 files changed, 100 insertions(+) create mode 100644 includes/sanitizers/class-amp-core-theme-sanitizer.php diff --git a/includes/class-amp-autoloader.php b/includes/class-amp-autoloader.php index c86307f0a77..ea49075b1b8 100644 --- a/includes/class-amp-autoloader.php +++ b/includes/class-amp-autoloader.php @@ -78,6 +78,7 @@ class AMP_Autoloader { 'AMP_Style_Sanitizer' => 'includes/sanitizers/class-amp-style-sanitizer', 'AMP_Tag_And_Attribute_Sanitizer' => 'includes/sanitizers/class-amp-tag-and-attribute-sanitizer', 'AMP_Video_Sanitizer' => 'includes/sanitizers/class-amp-video-sanitizer', + 'AMP_Core_Theme_Sanitizer' => 'includes/sanitizers/class-amp-core-theme-sanitizer', 'AMP_Customizer_Design_Settings' => 'includes/settings/class-amp-customizer-design-settings', 'AMP_Customizer_Settings' => 'includes/settings/class-amp-customizer-settings', 'AMP_Content' => 'includes/templates/class-amp-content', diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 67fad611c80..f9c200662cb 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -214,6 +214,14 @@ public static function register_paired_hooks() { */ public static function add_hooks() { + add_filter( 'amp_content_sanitizers', function( $sanitizers ) { + $sanitizers['AMP_Core_Theme_Sanitizer'] = array( + 'template' => get_template(), + 'stylesheet' => get_stylesheet(), + ); + return $sanitizers; + } ); + // Remove core actions which are invalid AMP. remove_action( 'wp_head', 'wp_post_preview_js', 1 ); remove_action( 'wp_head', 'print_emoji_detection_script', 7 ); diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php new file mode 100644 index 00000000000..d3212c4b88c --- /dev/null +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -0,0 +1,91 @@ + array( + 'force_svg_support' => array(), + 'force_fixed_background_support' => array(), + // @todo Header video probably needs special support in the plugin generally. + // @todo Header image is not styled properly. Needs layout=responsive and other things? + // @todo Dequeue scripts and replace with AMP functionality where possible. + ), + ); + + /** + * Fix up core themes to do things in the AMP way. + * + * @since 1.0 + */ + public function sanitize() { + $theme_features = array(); + + // Find theme features for core theme. + $theme_candidates = wp_array_slice_assoc( $this->args, array( 'stylesheet', 'template' ) ); + foreach ( $theme_candidates as $theme_candidate ) { + if ( isset( $this->theme_features[ $theme_candidate ] ) ) { + $theme_features = $this->theme_features[ $theme_candidate ]; + break; + } + } + + // Allow specific theme features to be requested even if the theme is not in core. + if ( isset( $this->args['theme_features'] ) ) { + $theme_features = array_merge( $this->args['theme_features'], $theme_features ); + } + + foreach ( $theme_features as $theme_feature => $feature_args ) { + if ( method_exists( $this, $theme_feature ) ) { + call_user_func( array( $this, $theme_feature ), $feature_args ); + } + } + } + + /** + * Force SVG support, replacing no-svg class name with svg class name. + * + * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/assets/js/global.js#L211-L213 + * @link https://caniuse.com/#feat=svg + */ + public function force_svg_support() { + $this->dom->documentElement->setAttribute( + 'class', + preg_replace( + '/(^|\s)no-svg(\s|$)/', + ' svg ', + $this->dom->documentElement->getAttribute( 'class' ) + ) + ); + } + + /** + * Force support for fixed background-attachment. + * + * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/assets/js/global.js#L215-L217 + * @link https://caniuse.com/#feat=background-attachment + */ + public function force_fixed_background_support() { + $this->dom->documentElement->setAttribute( + 'class', + $this->dom->documentElement->getAttribute( 'class' ) . ' background-fixed' + ); + } +} From 469257764ef56820efb99b5575234bdf19d47b3e Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Wed, 16 May 2018 21:14:44 -0500 Subject: [PATCH 02/27] Begin AMP header video implementation. This is still WIP, free for someone to work on. @todo: as Weston mentioned, this needs to support Vimeo. --- includes/class-amp-theme-support.php | 58 ++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 97d76280738..e3b9cedc889 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -281,6 +281,7 @@ public static function add_hooks() { add_action( 'comment_form', array( __CLASS__, 'amend_comment_form' ), 100 ); remove_action( 'comment_form', 'wp_comment_form_unfiltered_html_nonce' ); add_filter( 'wp_kses_allowed_html', array( __CLASS__, 'whitelist_layout_in_wp_kses_allowed_html' ), 10 ); + add_filter( 'get_header_image_tag', array( __CLASS__, 'conditionally_output_header_video' ) ); // @todo Add character conversion. } @@ -1172,4 +1173,61 @@ public static function enqueue_assets() { // Enqueue default styles expected by sanitizer. wp_enqueue_style( 'amp-default', amp_get_asset_url( 'css/amp-default.css' ), array(), AMP__VERSION ); } + + /** + * Conditionally replace the header image markup with a header video. + * + * This is JS-driven in Core themes like Twenty Sixteen and Twenty Seventeen. + * So in order for the header video to display, + * this replaces the markup of the header image. + * + * @param string $html The image markup to filter. + * @return string $html Filtered markup. + */ + public static function conditionally_output_header_video( $html ) { + if ( ! is_header_video_active() || ! ( has_header_video() || is_customize_preview() ) ) { + return $html; + } + + $video_settings = get_header_video_settings(); + if ( ! isset( $video_settings['videoUrl'], $video_settings['width'], $video_settings['height'] ) ) { + return $html; + } + + $parsed_url = wp_parse_url( $video_settings['videoUrl'] ); + $query = isset( $parsed_url['query'] ) ? wp_parse_args( $parsed_url['query'] ) : null; + $video_attributes = array( + 'width' => $video_settings['width'], + 'height' => $video_settings['height'], + 'layout' => 'responsive', + 'autoplay' => '', + 'id' => 'wp-custom-header-video', + ); + + // If the video URL is for YouTube, return an element. + if ( isset( $parsed_url['host'], $query['v'] ) && ( false !== strpos( $parsed_url['host'], 'youtube' ) ) ) { + return AMP_HTML_Utils::build_tag( + 'amp-youtube', + array_merge( + $video_attributes, + array( + 'data-videoid' => $query['v'], + 'data-param-rel' => '0', // Don't show related videos. + 'data-param-showinfo' => '0', // Don't show video title at the top. + ) + ) + ); + } else { + return AMP_HTML_Utils::build_tag( + 'amp-video', + array_merge( + $video_attributes, + array( + 'src' => $video_settings['videoUrl'], + ) + ) + ); + } + } + } From fc0bea1331dafc2275347ab8a8b716e57d2ccd0b Mon Sep 17 00:00:00 2001 From: Ryan Kienstra Date: Tue, 29 May 2018 01:46:44 -0500 Subject: [PATCH 03/27] Begin PHPUnit test of AMP header video implementation. Also, simplify the conditionals at the top of conditionally_output_header_video(). @todo: test the case where the header video is an , not an . --- includes/class-amp-theme-support.php | 6 ++++-- tests/test-class-amp-theme-support.php | 29 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index cad2de77ee4..6fa7a5dc468 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1216,13 +1216,15 @@ public static function enqueue_assets() { * So in order for the header video to display, * this replaces the markup of the header image. * + * @since 1.0 + * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 * @param string $html The image markup to filter. * @return string $html Filtered markup. */ public static function conditionally_output_header_video( $html ) { - if ( ! is_header_video_active() || ! ( has_header_video() || is_customize_preview() ) ) { + if ( ! is_header_video_active() ) { return $html; - } + }; $video_settings = get_header_video_settings(); if ( ! isset( $video_settings['videoUrl'], $video_settings['width'], $video_settings['height'] ) ) { diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 04278d0f8ce..0965beceb09 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -30,6 +30,7 @@ public function tearDown() { $wp_scripts = null; parent::tearDown(); remove_theme_support( 'amp' ); + remove_theme_support( 'custom-header' ); $_REQUEST = array(); // phpcs:ignore WordPress.CSRF.NonceVerification.NoNonceVerification $_SERVER['QUERY_STRING'] = ''; unset( $_SERVER['REQUEST_URI'] ); @@ -287,6 +288,7 @@ public function test_add_hooks() { $this->assertEquals( 10, has_filter( 'cancel_comment_reply_link', array( self::TESTED_CLASS, 'filter_cancel_comment_reply_link' ) ) ); $this->assertEquals( 100, has_action( 'comment_form', array( self::TESTED_CLASS, 'amend_comment_form' ) ) ); $this->assertFalse( has_action( 'comment_form', 'wp_comment_form_unfiltered_html_nonce' ) ); + $this->assertEquals( 10, has_filter( 'get_header_image_tag', array( self::TESTED_CLASS, 'conditionally_output_header_video' ) ) ); } /** @@ -1208,4 +1210,31 @@ public function test_whitelist_layout_in_wp_kses_allowed_html() { $image = ''; $this->assertEquals( $image, wp_kses_post( $image ) ); } + + /** + * Test AMP_Theme_Support::conditionally_output_header_video(). + * + * @see AMP_Theme_Support::conditionally_output_header_video() + */ + public function test_conditionally_output_header_video() { + $mock_image = ''; + + // If there's no theme support for 'custom-header', the callback should simply return the image. + $this->assertEquals( + $mock_image, + AMP_Theme_Support::conditionally_output_header_video( $mock_image ) + ); + + // If theme support is present, but there isn't a header video selected, the callback should again return the image. + add_theme_support( 'custom-header', array( + 'video' => true, + ) ); + + // There's a YouTube URL as the header video. + set_theme_mod( 'external_header_video', 'https://www.youtube.com/watch?v=a8NScvBhVnc' ); + $this->assertEquals( + '', + AMP_Theme_Support::conditionally_output_header_video( $mock_image ) + ); + } } From 7f738c114bde2d2f37ad1bdd52b3d0ee3e6c4603 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 30 May 2018 15:51:37 +0200 Subject: [PATCH 04/27] create image and video image tags in separate methods --- includes/class-amp-theme-support.php | 63 ++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 9 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 6fa7a5dc468..8fd34d2f631 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -288,7 +288,7 @@ public static function add_hooks() { add_action( 'comment_form', array( __CLASS__, 'amend_comment_form' ), 100 ); remove_action( 'comment_form', 'wp_comment_form_unfiltered_html_nonce' ); add_filter( 'wp_kses_allowed_html', array( __CLASS__, 'whitelist_layout_in_wp_kses_allowed_html' ), 10 ); - add_filter( 'get_header_image_tag', array( __CLASS__, 'conditionally_output_header_video' ) ); + add_filter( 'get_header_image_tag', array( __CLASS__, 'conditionally_output_header' ), 10, 3 ); // @todo Add character conversion. } @@ -1210,7 +1210,7 @@ public static function enqueue_assets() { } /** - * Conditionally replace the header image markup with a header video. + * Conditionally replace the header image markup with a header video or image. * * This is JS-driven in Core themes like Twenty Sixteen and Twenty Seventeen. * So in order for the header video to display, @@ -1221,29 +1221,51 @@ public static function enqueue_assets() { * @param string $html The image markup to filter. * @return string $html Filtered markup. */ - public static function conditionally_output_header_video( $html ) { + public static function conditionally_output_header( $html, $header, $atts ) { if ( ! is_header_video_active() ) { return $html; }; - $video_settings = get_header_video_settings(); - if ( ! isset( $video_settings['videoUrl'], $video_settings['width'], $video_settings['height'] ) ) { - return $html; + if ( ! has_header_video() ) { + return self::output_header_image( $atts ); } + return self::output_header_video( $atts ); + } + + /** + * Replace the header image markup with a header video. + * + * This is JS-driven in Core themes like Twenty Sixteen and Twenty Seventeen. + * So in order for the header video to display, + * this replaces the markup of the header image. + * + * @since 1.0 + * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 + * @param array $atts The header tag attributes array. + * @return string $html Filtered markup. + */ + public static function output_header_video( $atts ) { + + $video_settings = get_header_video_settings(); + $parsed_url = wp_parse_url( $video_settings['videoUrl'] ); $query = isset( $parsed_url['query'] ) ? wp_parse_args( $parsed_url['query'] ) : null; $video_attributes = array( + 'media' => '(min-width: ' . $video_settings['minWidth'] . 'px)', 'width' => $video_settings['width'], 'height' => $video_settings['height'], - 'layout' => 'responsive', + 'layout' => 'fill', 'autoplay' => '', 'id' => 'wp-custom-header-video', ); + $atts['placeholder'] = ''; + $image_placeholder = self::output_header_image( $atts ); + // If the video URL is for YouTube, return an element. if ( isset( $parsed_url['host'], $query['v'] ) && ( false !== strpos( $parsed_url['host'], 'youtube' ) ) ) { - return AMP_HTML_Utils::build_tag( + $video_header = AMP_HTML_Utils::build_tag( 'amp-youtube', array_merge( $video_attributes, @@ -1255,7 +1277,7 @@ public static function conditionally_output_header_video( $html ) { ) ); } else { - return AMP_HTML_Utils::build_tag( + $video_header = AMP_HTML_Utils::build_tag( 'amp-video', array_merge( $video_attributes, @@ -1265,6 +1287,29 @@ public static function conditionally_output_header_video( $html ) { ) ); } + return $image_placeholder . $video_header; } + /** + * Replace the header image markup with a header image. + * + * This is JS-driven in Core themes like Twenty Sixteen and Twenty Seventeen. + * So in order for the header video to display, + * this replaces the markup of the header image. + * + * @since 1.0 + * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 + * @param array $atts The image tag attributes. + * @return string $html Filtered markup. + */ + public static function output_header_image( $atts ) { + + $atts['layout'] = 'fill'; + unset( $atts['width'] ); + + $place_holder = AMP_HTML_Utils::build_tag( 'amp-img', $atts ); + + return $place_holder; + + } } From cbacd9d0400c3b0d93e1dc8a7558e06f5e05c881 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 30 May 2018 15:58:08 +0200 Subject: [PATCH 05/27] add doc params --- includes/class-amp-theme-support.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 8fd34d2f631..4dcb40c93cf 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1219,6 +1219,8 @@ public static function enqueue_assets() { * @since 1.0 * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 * @param string $html The image markup to filter. + * @param array $header The header config array. + * @param array $atts The image markup attributes. * @return string $html Filtered markup. */ public static function conditionally_output_header( $html, $header, $atts ) { From 31cb15498ef3fdc08058d258fc7ac8ebc895acb5 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 30 May 2018 16:39:05 +0200 Subject: [PATCH 06/27] fix unit tests for video header --- tests/test-class-amp-theme-support.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 0965beceb09..9dfe9c6aef4 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -288,7 +288,7 @@ public function test_add_hooks() { $this->assertEquals( 10, has_filter( 'cancel_comment_reply_link', array( self::TESTED_CLASS, 'filter_cancel_comment_reply_link' ) ) ); $this->assertEquals( 100, has_action( 'comment_form', array( self::TESTED_CLASS, 'amend_comment_form' ) ) ); $this->assertFalse( has_action( 'comment_form', 'wp_comment_form_unfiltered_html_nonce' ) ); - $this->assertEquals( 10, has_filter( 'get_header_image_tag', array( self::TESTED_CLASS, 'conditionally_output_header_video' ) ) ); + $this->assertEquals( 10, has_filter( 'get_header_image_tag', array( self::TESTED_CLASS, 'conditionally_output_header' ) ) ); } /** @@ -1048,7 +1048,7 @@ public function test_prepare_response() { $this->assertContains( '', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript $this->assertContains( '', $sanitized_html ); - $this->assertCount( 3, $removed_nodes ); + $this->assertCount( 4, $removed_nodes ); $this->assertInstanceOf( 'DOMElement', $removed_nodes['script'] ); $this->assertInstanceOf( 'DOMAttr', $removed_nodes['onclick'] ); $this->assertInstanceOf( 'DOMAttr', $removed_nodes['handle'] ); @@ -1212,17 +1212,17 @@ public function test_whitelist_layout_in_wp_kses_allowed_html() { } /** - * Test AMP_Theme_Support::conditionally_output_header_video(). + * Test AMP_Theme_Support::conditionally_output_header(). * - * @see AMP_Theme_Support::conditionally_output_header_video() + * @see AMP_Theme_Support::conditionally_output_header() */ - public function test_conditionally_output_header_video() { + public function conditionally_output_header() { $mock_image = ''; // If there's no theme support for 'custom-header', the callback should simply return the image. $this->assertEquals( $mock_image, - AMP_Theme_Support::conditionally_output_header_video( $mock_image ) + AMP_Theme_Support::conditionally_output_header( $mock_image ) ); // If theme support is present, but there isn't a header video selected, the callback should again return the image. @@ -1234,7 +1234,7 @@ public function test_conditionally_output_header_video() { set_theme_mod( 'external_header_video', 'https://www.youtube.com/watch?v=a8NScvBhVnc' ); $this->assertEquals( '', - AMP_Theme_Support::conditionally_output_header_video( $mock_image ) + AMP_Theme_Support::conditionally_output_header( $mock_image ) ); } } From a36a281de57aeee1201312e6ab340c5162718ec5 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Wed, 30 May 2018 16:46:23 +0200 Subject: [PATCH 07/27] correct expected count in unit test --- tests/test-class-amp-theme-support.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 9dfe9c6aef4..d5994a40c3c 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -1048,7 +1048,7 @@ public function test_prepare_response() { $this->assertContains( '', $sanitized_html ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript $this->assertContains( '', $sanitized_html ); - $this->assertCount( 4, $removed_nodes ); + $this->assertCount( 3, $removed_nodes ); $this->assertInstanceOf( 'DOMElement', $removed_nodes['script'] ); $this->assertInstanceOf( 'DOMAttr', $removed_nodes['onclick'] ); $this->assertInstanceOf( 'DOMAttr', $removed_nodes['handle'] ); From 601c0053c00ecb449e09e8b8eacc86c396639628 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Thu, 31 May 2018 12:05:11 +0200 Subject: [PATCH 08/27] remove video script enqueue --- includes/class-amp-theme-support.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 81b113cdce3..c8b5518a228 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1292,7 +1292,8 @@ public static function conditionally_output_header( $html, $header, $atts ) { * @return string $html Filtered markup. */ public static function output_header_video( $atts ) { - + // Remove the script for video. + wp_deregister_script( 'wp-custom-header' ); $video_settings = get_header_video_settings(); $parsed_url = wp_parse_url( $video_settings['videoUrl'] ); @@ -1319,6 +1320,7 @@ public static function output_header_video( $atts ) { 'data-videoid' => $query['v'], 'data-param-rel' => '0', // Don't show related videos. 'data-param-showinfo' => '0', // Don't show video title at the top. + 'data-param-controls' => '0', // Don't show video controls. ) ) ); @@ -1353,7 +1355,7 @@ public static function output_header_image( $atts ) { $atts['layout'] = 'fill'; unset( $atts['width'] ); - $place_holder = AMP_HTML_Utils::build_tag( 'amp-img', $atts ); + $place_holder = AMP_HTML_Utils::build_tag( 'img', $atts ); return $place_holder; From dc9e628d92bf2643543d58e261a6834f2b96c014 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 1 Jun 2018 10:24:53 +0200 Subject: [PATCH 09/27] Add required styles for amp components in the header image and video --- includes/class-amp-theme-support.php | 15 ++-- .../class-amp-core-theme-sanitizer.php | 71 ++++++++++++++++++- .../sanitizers/class-amp-video-sanitizer.php | 2 +- 3 files changed, 77 insertions(+), 11 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index c8b5518a228..1a08ae28584 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1288,6 +1288,7 @@ public static function conditionally_output_header( $html, $header, $atts ) { * * @since 1.0 * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 + * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L78 * @param array $atts The header tag attributes array. * @return string $html Filtered markup. */ @@ -1302,13 +1303,13 @@ public static function output_header_video( $atts ) { 'media' => '(min-width: ' . $video_settings['minWidth'] . 'px)', 'width' => $video_settings['width'], 'height' => $video_settings['height'], - 'layout' => 'fill', + 'layout' => 'responsive', 'autoplay' => '', 'id' => 'wp-custom-header-video', ); - $atts['placeholder'] = ''; - $image_placeholder = self::output_header_image( $atts ); + // Create image banner to stay behind the video. + $image_header = self::output_header_image( $atts ); // If the video URL is for YouTube, return an element. if ( isset( $parsed_url['host'], $query['v'] ) && ( false !== strpos( $parsed_url['host'], 'youtube' ) ) ) { @@ -1335,7 +1336,8 @@ public static function output_header_video( $atts ) { ) ); } - return $image_placeholder . $video_header; + + return $image_header . $video_header; } /** @@ -1352,10 +1354,7 @@ public static function output_header_video( $atts ) { */ public static function output_header_image( $atts ) { - $atts['layout'] = 'fill'; - unset( $atts['width'] ); - - $place_holder = AMP_HTML_Utils::build_tag( 'img', $atts ); + $place_holder = AMP_HTML_Utils::build_tag( 'amp-img', $atts ); return $place_holder; diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index d3212c4b88c..075aae31120 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -24,8 +24,7 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'twentyseventeen' => array( 'force_svg_support' => array(), 'force_fixed_background_support' => array(), - // @todo Header video probably needs special support in the plugin generally. - // @todo Header image is not styled properly. Needs layout=responsive and other things? + 'header_banner_styles' => array(), // @todo Dequeue scripts and replace with AMP functionality where possible. ), ); @@ -88,4 +87,72 @@ public function force_fixed_background_support() { $this->dom->documentElement->getAttribute( 'class' ) . ' background-fixed' ); } + + /** + * Add required styles for video and image headers. + * + * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1687 + * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1743 + */ + public function header_banner_styles() { + + $body = $this->dom->getElementsByTagName( 'body' )->item( 0 ); + if ( has_header_video() ) { + $body->setAttribute( + 'class', + $body->getAttribute( 'class' ) . ' has-header-video' + ); + } + + $style_element = $this->dom->createElement( 'style' ); + $style_content = ' + .has-header-image .custom-header-media amp-img > img, + .has-header-video .custom-header-media amp-video > video, + .has-header-video .custom-header-media amp-youtube > iframe { + position: fixed; + height: auto; + left: 50%; + max-width: 1000%; + min-height: 100%; + min-width: 100%; + min-width: 100vw; /* vw prevents 1px gap on left that 100% has */ + width: auto; + top: 50%; + padding-bottom: 1px; /* Prevent header from extending beyond the footer */ + -ms-transform: translateX(-50%) translateY(-50%); + -moz-transform: translateX(-50%) translateY(-50%); + -webkit-transform: translateX(-50%) translateY(-50%); + transform: translateX(-50%) translateY(-50%); + } + .has-header-image:not(.twentyseventeen-front-page):not(.home) .custom-header-media amp-img > img { + bottom: 0; + position: absolute; + top: auto; + -ms-transform: translateX(-50%) translateY(0); + -moz-transform: translateX(-50%) translateY(0); + -webkit-transform: translateX(-50%) translateY(0); + transform: translateX(-50%) translateY(0); + } + /* For browsers that support \'object-fit\' */ + @supports ( object-fit: cover ) { + .has-header-image .custom-header-media amp-img > img, + .has-header-video .custom-header-media amp-video > video, + .has-header-video .custom-header-media amp-youtube > iframe, + .has-header-image:not(.twentyseventeen-front-page):not(.home) .custom-header-media amp-img > img { + height: 100%; + left: 0; + -o-object-fit: cover; + object-fit: cover; + top: 0; + -ms-transform: none; + -moz-transform: none; + -webkit-transform: none; + transform: none; + width: 100%; + } + } + '; + $style_element->appendChild( $this->dom->createTextNode( $style_content ) ); + $body->appendChild( $style_element ); + } } diff --git a/includes/sanitizers/class-amp-video-sanitizer.php b/includes/sanitizers/class-amp-video-sanitizer.php index b61c85343fb..668dfed9064 100644 --- a/includes/sanitizers/class-amp-video-sanitizer.php +++ b/includes/sanitizers/class-amp-video-sanitizer.php @@ -39,7 +39,7 @@ class AMP_Video_Sanitizer extends AMP_Base_Sanitizer { */ public function get_selector_conversion_mapping() { return array( - 'video' => array( 'amp-video' ), + 'video' => array( 'amp-video', 'amp-youtube' ), ); } From 54e025a932ecff69d9916d16940a4f33f39e4b46 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Fri, 1 Jun 2018 10:57:44 +0200 Subject: [PATCH 10/27] remove unused amp-youtube as styles work correctly --- includes/sanitizers/class-amp-core-theme-sanitizer.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 075aae31120..e00eddb9978 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -107,8 +107,7 @@ public function header_banner_styles() { $style_element = $this->dom->createElement( 'style' ); $style_content = ' .has-header-image .custom-header-media amp-img > img, - .has-header-video .custom-header-media amp-video > video, - .has-header-video .custom-header-media amp-youtube > iframe { + .has-header-video .custom-header-media amp-video > video{ position: fixed; height: auto; left: 50%; @@ -137,7 +136,6 @@ public function header_banner_styles() { @supports ( object-fit: cover ) { .has-header-image .custom-header-media amp-img > img, .has-header-video .custom-header-media amp-video > video, - .has-header-video .custom-header-media amp-youtube > iframe, .has-header-image:not(.twentyseventeen-front-page):not(.home) .custom-header-media amp-img > img { height: 100%; left: 0; From a91f0e28a358ffbe21ec3bb5b3b195a825fbad62 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 2 Jun 2018 16:16:48 -0700 Subject: [PATCH 11/27] Rename header_banner_styles to add_header_media_styles --- .../class-amp-core-theme-sanitizer.php | 104 ++++++++++-------- 1 file changed, 61 insertions(+), 43 deletions(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index e00eddb9978..1b4d0ddd95e 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -15,6 +15,17 @@ */ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { + /** + * Array of flags used to control sanitization. + * + * @var array { + * @type string $stylesheet Stylesheet slug. + * @type string $template Template slug. + * @type array $theme_features List of theme features that need to be applied. Features are method names, + * } + */ + protected $args; + /** * Config for features needed by themes. * @@ -24,7 +35,7 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'twentyseventeen' => array( 'force_svg_support' => array(), 'force_fixed_background_support' => array(), - 'header_banner_styles' => array(), + 'add_header_media_styles' => array(), // @todo Dequeue scripts and replace with AMP functionality where possible. ), ); @@ -91,10 +102,12 @@ public function force_fixed_background_support() { /** * Add required styles for video and image headers. * + * This is currently used exclusively for Twenty Seventeen. + * * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1687 * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1743 */ - public function header_banner_styles() { + public function add_header_media_styles() { $body = $this->dom->getElementsByTagName( 'body' )->item( 0 ); if ( has_header_video() ) { @@ -104,53 +117,58 @@ public function header_banner_styles() { ); } + /* + * The following is necessary because the styles in the theme apply to img and video, + * and the CSS parser will then convert the selectors to amp-img and amp-video respectively. + * Nevertheless, object-fit does not apply on amp-img and it needs to apply on an actual img. + */ $style_element = $this->dom->createElement( 'style' ); $style_content = ' - .has-header-image .custom-header-media amp-img > img, - .has-header-video .custom-header-media amp-video > video{ - position: fixed; - height: auto; - left: 50%; - max-width: 1000%; - min-height: 100%; - min-width: 100%; - min-width: 100vw; /* vw prevents 1px gap on left that 100% has */ - width: auto; - top: 50%; - padding-bottom: 1px; /* Prevent header from extending beyond the footer */ - -ms-transform: translateX(-50%) translateY(-50%); - -moz-transform: translateX(-50%) translateY(-50%); - -webkit-transform: translateX(-50%) translateY(-50%); - transform: translateX(-50%) translateY(-50%); - } - .has-header-image:not(.twentyseventeen-front-page):not(.home) .custom-header-media amp-img > img { - bottom: 0; - position: absolute; - top: auto; - -ms-transform: translateX(-50%) translateY(0); - -moz-transform: translateX(-50%) translateY(0); - -webkit-transform: translateX(-50%) translateY(0); - transform: translateX(-50%) translateY(0); - } - /* For browsers that support \'object-fit\' */ - @supports ( object-fit: cover ) { .has-header-image .custom-header-media amp-img > img, - .has-header-video .custom-header-media amp-video > video, + .has-header-video .custom-header-media amp-video > video{ + position: fixed; + height: auto; + left: 50%; + max-width: 1000%; + min-height: 100%; + min-width: 100%; + min-width: 100vw; /* vw prevents 1px gap on left that 100% has */ + width: auto; + top: 50%; + padding-bottom: 1px; /* Prevent header from extending beyond the footer */ + -ms-transform: translateX(-50%) translateY(-50%); + -moz-transform: translateX(-50%) translateY(-50%); + -webkit-transform: translateX(-50%) translateY(-50%); + transform: translateX(-50%) translateY(-50%); + } .has-header-image:not(.twentyseventeen-front-page):not(.home) .custom-header-media amp-img > img { - height: 100%; - left: 0; - -o-object-fit: cover; - object-fit: cover; - top: 0; - -ms-transform: none; - -moz-transform: none; - -webkit-transform: none; - transform: none; - width: 100%; + bottom: 0; + position: absolute; + top: auto; + -ms-transform: translateX(-50%) translateY(0); + -moz-transform: translateX(-50%) translateY(0); + -webkit-transform: translateX(-50%) translateY(0); + transform: translateX(-50%) translateY(0); + } + /* For browsers that support object-fit */ + @supports ( object-fit: cover ) { + .has-header-image .custom-header-media amp-img > img, + .has-header-video .custom-header-media amp-video > video, + .has-header-image:not(.twentyseventeen-front-page):not(.home) .custom-header-media amp-img > img { + height: 100%; + left: 0; + -o-object-fit: cover; + object-fit: cover; + top: 0; + -ms-transform: none; + -moz-transform: none; + -webkit-transform: none; + transform: none; + width: 100%; + } } - } '; $style_element->appendChild( $this->dom->createTextNode( $style_content ) ); - $body->appendChild( $style_element ); + $body->appendChild( $style_element ); // Note that AMP_Style_Sanitizer will take the CSS and put it in style[amp-custom]. } } From e47275c0bd4e254199ff3a75046a6bf099f73325 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 2 Jun 2018 16:31:39 -0700 Subject: [PATCH 12/27] Split out add_has_header_video_body_class feature as separate method --- .../class-amp-core-theme-sanitizer.php | 51 ++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 1b4d0ddd95e..d4900be7de8 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -26,6 +26,13 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { */ protected $args; + /** + * Body element. + * + * @var DOMElement + */ + protected $body; + /** * Config for features needed by themes. * @@ -33,9 +40,10 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { */ public $theme_features = array( 'twentyseventeen' => array( - 'force_svg_support' => array(), - 'force_fixed_background_support' => array(), - 'add_header_media_styles' => array(), + 'force_svg_support' => array(), + 'force_fixed_background_support' => array(), + 'fixup_twentyseventeen_masthead' => array(), + 'add_has_header_video_body_class' => array(), // @todo Dequeue scripts and replace with AMP functionality where possible. ), ); @@ -48,6 +56,11 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { public function sanitize() { $theme_features = array(); + $this->body = $this->dom->getElementsByTagName( 'body' )->item( 0 ); + if ( ! $this->body ) { + return; + } + // Find theme features for core theme. $theme_candidates = wp_array_slice_assoc( $this->args, array( 'stylesheet', 'template' ) ); foreach ( $theme_candidates as $theme_candidate ) { @@ -100,23 +113,37 @@ public function force_fixed_background_support() { } /** - * Add required styles for video and image headers. + * Add body class when there is a header video. * - * This is currently used exclusively for Twenty Seventeen. + * @link https://github.com/WordPress/wordpress-develop/blob/a26c24226c6b131a0ed22c722a836c100d3ba254/src/wp-content/themes/twentyseventeen/assets/js/global.js#L244-L247 * - * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1687 - * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1743 + * @param array $args Args. */ - public function add_header_media_styles() { + public function add_has_header_video_body_class( $args = array() ) { + $args = array_merge( + array( + 'class_name' => 'has-header-video', + ), + $args + ); - $body = $this->dom->getElementsByTagName( 'body' )->item( 0 ); if ( has_header_video() ) { - $body->setAttribute( + $this->body->setAttribute( 'class', - $body->getAttribute( 'class' ) . ' has-header-video' + $this->body->getAttribute( 'class' ) . ' ' . $args['class_name'] ); } + } + /** + * Add required styles for video and image headers. + * + * This is currently used exclusively for Twenty Seventeen. + * + * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1687 + * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1743 + */ + public function fixup_twentyseventeen_masthead() { /* * The following is necessary because the styles in the theme apply to img and video, * and the CSS parser will then convert the selectors to amp-img and amp-video respectively. @@ -169,6 +196,6 @@ public function add_header_media_styles() { } '; $style_element->appendChild( $this->dom->createTextNode( $style_content ) ); - $body->appendChild( $style_element ); // Note that AMP_Style_Sanitizer will take the CSS and put it in style[amp-custom]. + $this->body->appendChild( $style_element ); // Note that AMP_Style_Sanitizer will take the CSS and put it in style[amp-custom]. } } From d41e498a1b14d82449a81e8ea07c0549939a476d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 3 Jun 2018 00:02:28 -0700 Subject: [PATCH 13/27] Add mechanism to allow sanitizers to add hooks during output buffering Implement support for mobile nav menus. --- includes/class-amp-theme-support.php | 6 + .../sanitizers/class-amp-base-sanitizer.php | 16 + .../class-amp-core-theme-sanitizer.php | 394 ++++++++++++++++-- phpcs.xml | 2 +- 4 files changed, 384 insertions(+), 34 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index b7228cddac9..2df0f55dda4 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -159,6 +159,12 @@ public static function finish_init() { self::$sanitizer_classes = amp_get_content_sanitizers(); self::$sanitizer_classes = AMP_Validation_Manager::filter_sanitizer_args( self::$sanitizer_classes ); self::$embed_handlers = self::register_content_embed_handlers(); + + foreach ( self::$sanitizer_classes as $sanitizer_class => $args ) { + if ( method_exists( $sanitizer_class, 'add_buffering_hooks' ) ) { + call_user_func( array( $sanitizer_class, 'add_buffering_hooks' ), $args ); + } + } } /** diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index 71aaec157a6..55e42505339 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -121,6 +121,22 @@ public function __construct( $dom, $args = array() ) { } } + /** + * Add filters to manipulate output during output buffering before the DOM is constructed. + * + * Add actions and filters before the page is rendered so that the sanitizer can fix issues during output buffering. + * This provides an alternative to manipulating the DOM in the sanitize method. This is a static function because + * it is invoked before the class is instantiated, as the DOM is not available yet. This method is only called + * when 'amp' theme support is present. It is conceptually similar to the AMP_Base_Embed_Handler class's register_embed + * method. + * + * @since 1.0 + * @see \AMP_Base_Embed_Handler::register_embed() + * + * @param array $args Args. + */ + public static function add_buffering_hooks( $args = array() ) {} + /** * Get mapping of HTML selectors to the AMP component selectors which they may be converted into. * diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index d4900be7de8..9ef9818c760 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -33,48 +33,105 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { */ protected $body; + /** + * XPath. + * + * @var DOMXPath + */ + protected $xpath; + /** * Config for features needed by themes. * * @var array */ - public $theme_features = array( + protected static $theme_features = array( 'twentyseventeen' => array( - 'force_svg_support' => array(), - 'force_fixed_background_support' => array(), - 'fixup_twentyseventeen_masthead' => array(), - 'add_has_header_video_body_class' => array(), + 'force_svg_support' => array(), + 'force_fixed_background_support' => array(), + 'add_twentyseventeen_masthead_styles' => array(), + 'add_has_header_video_body_class' => array(), + 'add_nav_menu_styles' => array(), + 'add_nav_menu_toggle' => array(), + 'add_nav_sub_menu_buttons' => array(), // @todo Dequeue scripts and replace with AMP functionality where possible. ), + 'twentyfifteen' => array( + 'add_nav_menu_styles' => array(), + 'add_nav_menu_toggle' => array(), + 'add_nav_sub_menu_buttons' => array(), + ), ); + /** + * Find theme features for core theme. + * + * @param array $args Args. + * @param bool $static Static. that is, whether should run during output buffering. + * @return array Theme features. + */ + protected static function get_theme_features( $args, $static = false ) { + $theme_features = array(); + $theme_candidates = wp_array_slice_assoc( $args, array( 'stylesheet', 'template' ) ); + foreach ( $theme_candidates as $theme_candidate ) { + if ( isset( self::$theme_features[ $theme_candidate ] ) ) { + $theme_features = self::$theme_features[ $theme_candidate ]; + break; + } + } + + // Allow specific theme features to be requested even if the theme is not in core. + if ( isset( $args['theme_features'] ) ) { + $theme_features = array_merge( $args['theme_features'], $theme_features ); + } + + $final_theme_features = array(); + foreach ( $theme_features as $theme_feature => $feature_args ) { + if ( ! method_exists( __CLASS__, $theme_feature ) ) { + continue; + } + try { + $reflection = new ReflectionMethod( __CLASS__, $theme_feature ); + if ( $reflection->isStatic() === $static ) { + $final_theme_features[ $theme_feature ] = $feature_args; + } + } catch ( Exception $e ) { + unset( $e ); + } + } + return $final_theme_features; + } + + /** + * Add filters to manipulate output during output buffering before the DOM is constructed. + * + * @since 1.0 + * + * @param array $args Args. + */ + public static function add_buffering_hooks( $args = array() ) { + $theme_features = self::get_theme_features( $args, true ); + foreach ( $theme_features as $theme_feature => $feature_args ) { + if ( method_exists( __CLASS__, $theme_feature ) ) { + call_user_func( array( __CLASS__, $theme_feature ), $feature_args ); + } + } + } + /** * Fix up core themes to do things in the AMP way. * * @since 1.0 */ public function sanitize() { - $theme_features = array(); - $this->body = $this->dom->getElementsByTagName( 'body' )->item( 0 ); if ( ! $this->body ) { return; } - // Find theme features for core theme. - $theme_candidates = wp_array_slice_assoc( $this->args, array( 'stylesheet', 'template' ) ); - foreach ( $theme_candidates as $theme_candidate ) { - if ( isset( $this->theme_features[ $theme_candidate ] ) ) { - $theme_features = $this->theme_features[ $theme_candidate ]; - break; - } - } - - // Allow specific theme features to be requested even if the theme is not in core. - if ( isset( $this->args['theme_features'] ) ) { - $theme_features = array_merge( $this->args['theme_features'], $theme_features ); - } + $this->xpath = new DOMXPath( $this->dom ); + $theme_features = self::get_theme_features( $this->args, false ); foreach ( $theme_features as $theme_feature => $feature_args ) { if ( method_exists( $this, $theme_feature ) ) { call_user_func( array( $this, $theme_feature ), $feature_args ); @@ -82,6 +139,52 @@ public function sanitize() { } } + /** + * Get theme config. + * + * @param string $theme Theme slug. + * @return array Class names. + */ + protected static function get_theme_config( $theme ) { + // phpcs:disable WordPress.WP.I18n.TextDomainMismatch + $config = array( + 'dropdown_class' => 'dropdown-toggle', + ); + switch ( $theme ) { + case 'twentyfifteen': + return array_merge( + $config, + array( + 'nav_container_id' => 'secondary', + 'nav_container_toggle_class' => 'toggled-on', + 'menu_button_class' => 'secondary-toggle', + 'menu_button_query' => '//header[ @id = "masthead" ]//button[ contains( @class, "secondary-toggle" ) ]', + 'menu_button_toggle_class' => 'toggled-on', + 'sub_menu_toggle_class' => 'toggle-on', + 'expand_text ' => __( 'expand child menu', 'twentyfifteen' ), + 'collapse_text' => __( 'collapse child menu', 'twentyfifteen' ), + ) + ); + + case 'twentyseventeen': + default: + return array_merge( + $config, + array( + 'nav_container_id' => 'site-navigation', + 'nav_container_toggle_class' => 'toggled-on', + 'menu_button_class' => 'menu-toggle', + 'menu_button_query' => '//nav[@id = "site-navigation"]//button[ contains( @class, "menu-toggle" ) ]', + 'menu_button_toggle_class' => 'toggled-on', + 'sub_menu_toggle_class' => 'toggled-on', + 'expand_text ' => __( 'expand child menu', 'twentyseventeen' ), + 'collapse_text' => __( 'collapse child menu', 'twentyseventeen' ), + ) + ); + } + // phpcs:enable WordPress.WP.I18n.TextDomainMismatch + } + /** * Force SVG support, replacing no-svg class name with svg class name. * @@ -119,7 +222,7 @@ public function force_fixed_background_support() { * * @param array $args Args. */ - public function add_has_header_video_body_class( $args = array() ) { + public static function add_has_header_video_body_class( $args = array() ) { $args = array_merge( array( 'class_name' => 'has-header-video', @@ -127,12 +230,12 @@ public function add_has_header_video_body_class( $args = array() ) { $args ); - if ( has_header_video() ) { - $this->body->setAttribute( - 'class', - $this->body->getAttribute( 'class' ) . ' ' . $args['class_name'] - ); - } + add_filter( 'body_class', function( $body_classes ) use ( $args ) { + if ( has_header_video() ) { + $body_classes[] = $args['class_name']; + } + return $body_classes; + } ); } /** @@ -143,14 +246,16 @@ public function add_has_header_video_body_class( $args = array() ) { * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1687 * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1743 */ - public function fixup_twentyseventeen_masthead() { + public static function add_twentyseventeen_masthead_styles() { /* * The following is necessary because the styles in the theme apply to img and video, * and the CSS parser will then convert the selectors to amp-img and amp-video respectively. * Nevertheless, object-fit does not apply on amp-img and it needs to apply on an actual img. */ - $style_element = $this->dom->createElement( 'style' ); - $style_content = ' + add_action( 'wp_enqueue_scripts', function() { + ob_start(); + ?> + + ', '' ), '', ob_get_clean() ); + wp_add_inline_style( get_template() . '-style', $styles ); + }, 11 ); + } + + /** + * Adjust header height. + * + * @todo Implement. + * @link https://github.com/WordPress/wordpress-develop/blob/a26c24226c6b131a0ed22c722a836c100d3ba254/src/wp-content/themes/twentyseventeen/assets/js/global.js#L88-L103 + */ + public function adjust_header_height() {} + + /** + * Add styles for the nav menu specifically to deal with AMP running in a no-js context. + * + * @param array $args Args. + */ + public static function add_nav_menu_styles( $args = array() ) { + $args = array_merge( + self::get_theme_config( get_template() ), + $args + ); + + add_action( 'wp_enqueue_scripts', function() use ( $args ) { + ob_start(); + ?> + + ', '' ), '', ob_get_clean() ); + wp_add_inline_style( get_template() . '-style', $styles ); + }, 11 ); + } + + /** + * Ensure that JS-only nav menu styles apply to AMP as well since even though scripts are not allowed, there are AMP-bind implementations. + * + * @param array $args Args. + */ + public function add_nav_menu_toggle( $args = array() ) { + $args = array_merge( + self::get_theme_config( get_template() ), + $args + ); + + $nav_el = $this->dom->getElementById( $args['nav_container_id'] ); + if ( ! $nav_el ) { + return; + } + + $button_el = $this->xpath->query( $args['menu_button_query'] )->item( 0 ); + if ( ! $button_el ) { + return; + } + + $state_id = 'navMenuToggledOn'; + $expanded = false; + + // @todo Not twentyfifteen? + $nav_el->setAttribute( + AMP_DOM_Utils::get_amp_bind_placeholder_prefix() . 'class', + sprintf( + "%s + ( $state_id ? %s : '' )", + wp_json_encode( $nav_el->getAttribute( 'class' ) ), + wp_json_encode( ' ' . $args['nav_container_toggle_class'] ) + ) + ); + + $state_el = $this->dom->createElement( 'amp-state' ); + $state_el->setAttribute( 'id', $state_id ); + $script_el = $this->dom->createElement( 'script' ); + $script_el->setAttribute( 'type', 'application/json' ); + $script_el->appendChild( $this->dom->createTextNode( wp_json_encode( $expanded ) ) ); + $state_el->appendChild( $script_el ); + $nav_el->parentNode->insertBefore( $state_el, $nav_el ); + + $button_on = sprintf( "tap:AMP.setState({ $state_id: ! $state_id })" ); + $button_el->setAttribute( 'on', $button_on ); + $button_el->setAttribute( 'aria-expanded', 'false' ); + $button_el->setAttribute( AMP_DOM_Utils::get_amp_bind_placeholder_prefix() . 'aria-expanded', "$state_id ? 'true' : 'false'" ); + $button_el->setAttribute( + AMP_DOM_Utils::get_amp_bind_placeholder_prefix() . 'class', + sprintf( "%s + ( $state_id ? %s : '' )", wp_json_encode( $button_el->getAttribute( 'class' ) ), wp_json_encode( ' ' . $args['menu_button_toggle_class'] ) ) + ); + } + + /** + * Add buttons for nav sub-menu items. + * + * @link https://github.com/WordPress/wordpress-develop/blob/a26c24226c6b131a0ed22c722a836c100d3ba254/src/wp-content/themes/twentyseventeen/assets/js/navigation.js#L11-L43 + * + * @param array $args Args. + */ + public static function add_nav_sub_menu_buttons( $args = array() ) { + $default_args = self::get_theme_config( get_template() ); + switch ( get_template() ) { + case 'twentyseventeen': + if ( function_exists( 'twentyseventeen_get_svg' ) ) { + $default_args['icon'] = twentyseventeen_get_svg( array( + 'icon' => 'angle-down', + 'fallback' => true, + ) ); + } + break; + } + $args = array_merge( $default_args, $args ); + + /** + * Filter the HTML output of a nav menu item to add the AMP dropdown button to reveal the sub-menu. + * + * @see twentyfifteen_amp_setup_hooks() + * + * @param string $item_output Nav menu item HTML. + * @param object $item Nav menu item. + * @return string Modified nav menu item HTML. + */ + add_filter( 'walker_nav_menu_start_el', function( $item_output, $item ) use ( $args ) { + if ( ! in_array( 'menu-item-has-children', $item->classes, true ) ) { + return $item_output; + } + static $nav_menu_item_number = 0; + $nav_menu_item_number++; + + $expanded = in_array( 'current-menu-ancestor', $item->classes, true ); + + $expanded_state_id = 'navMenuItemExpanded' . $nav_menu_item_number; + + // Create new state for managing storing the whether the sub-menu is expanded. + $item_output .= sprintf( + '', + esc_attr( $expanded_state_id ), + wp_json_encode( $expanded ) + ); + + $dropdown_button = '%s', + esc_attr( sprintf( "$expanded_state_id ? %s : %s", wp_json_encode( $args['collapse_text'] ), wp_json_encode( $args['expand_text'] ) ) ), + esc_html( $expanded ? $args['collapse_text'] : $args['expand_text'] ) + ); + } + + $dropdown_button .= ''; + + $item_output .= $dropdown_button; + return $item_output; + }, 10, 2 ); } } diff --git a/phpcs.xml b/phpcs.xml index fb1137e950a..ef3a5c5b010 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -28,7 +28,7 @@ - + From d3b8eae58a67c46412f0e35987a145073b00ca53 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 3 Jun 2018 09:34:02 -0700 Subject: [PATCH 14/27] Add nav menu support to Twenty Sixteen --- .../class-amp-core-theme-sanitizer.php | 117 +++++++++++------- 1 file changed, 75 insertions(+), 42 deletions(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 9ef9818c760..dc1bf18312d 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -56,6 +56,11 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_nav_sub_menu_buttons' => array(), // @todo Dequeue scripts and replace with AMP functionality where possible. ), + 'twentysixteen' => array( + 'add_nav_menu_styles' => array(), + 'add_nav_menu_toggle' => array(), + 'add_nav_sub_menu_buttons' => array(), + ), 'twentyfifteen' => array( 'add_nav_menu_styles' => array(), 'add_nav_menu_toggle' => array(), @@ -148,21 +153,36 @@ public function sanitize() { protected static function get_theme_config( $theme ) { // phpcs:disable WordPress.WP.I18n.TextDomainMismatch $config = array( - 'dropdown_class' => 'dropdown-toggle', + 'sub_menu_button_class' => 'dropdown-toggle', ); switch ( $theme ) { case 'twentyfifteen': return array_merge( $config, array( - 'nav_container_id' => 'secondary', - 'nav_container_toggle_class' => 'toggled-on', - 'menu_button_class' => 'secondary-toggle', - 'menu_button_query' => '//header[ @id = "masthead" ]//button[ contains( @class, "secondary-toggle" ) ]', - 'menu_button_toggle_class' => 'toggled-on', - 'sub_menu_toggle_class' => 'toggle-on', - 'expand_text ' => __( 'expand child menu', 'twentyfifteen' ), - 'collapse_text' => __( 'collapse child menu', 'twentyfifteen' ), + 'nav_container_id' => 'secondary', + 'nav_container_toggle_class' => 'toggled-on', + 'menu_button_class' => 'secondary-toggle', + 'menu_button_xpath' => '//header[ @id = "masthead" ]//button[ contains( @class, "secondary-toggle" ) ]', + 'menu_button_toggle_class' => 'toggled-on', + 'sub_menu_button_toggle_class' => 'toggle-on', + 'expand_text ' => __( 'expand child menu', 'twentyfifteen' ), + 'collapse_text' => __( 'collapse child menu', 'twentyfifteen' ), + ) + ); + + case 'twentysixteen': + return array_merge( + $config, + array( + 'nav_container_id' => 'site-header-menu', + 'nav_container_toggle_class' => 'toggled-on', + 'menu_button_class' => 'menu-toggle', + 'menu_button_xpath' => '//header[@id = "masthead"]//button[ @id = "menu-toggle" ]', + 'menu_button_toggle_class' => 'toggled-on', + 'sub_menu_button_toggle_class' => 'toggled-on', + 'expand_text ' => __( 'expand child menu', 'twentysixteen' ), + 'collapse_text' => __( 'collapse child menu', 'twentysixteen' ), ) ); @@ -171,14 +191,14 @@ protected static function get_theme_config( $theme ) { return array_merge( $config, array( - 'nav_container_id' => 'site-navigation', - 'nav_container_toggle_class' => 'toggled-on', - 'menu_button_class' => 'menu-toggle', - 'menu_button_query' => '//nav[@id = "site-navigation"]//button[ contains( @class, "menu-toggle" ) ]', - 'menu_button_toggle_class' => 'toggled-on', - 'sub_menu_toggle_class' => 'toggled-on', - 'expand_text ' => __( 'expand child menu', 'twentyseventeen' ), - 'collapse_text' => __( 'collapse child menu', 'twentyseventeen' ), + 'nav_container_id' => 'site-navigation', + 'nav_container_toggle_class' => 'toggled-on', + 'menu_button_class' => 'menu-toggle', + 'menu_button_xpath' => '//nav[@id = "site-navigation"]//button[ contains( @class, "menu-toggle" ) ]', + 'menu_button_toggle_class' => 'toggled-on', + 'sub_menu_button_toggle_class' => 'toggled-on', + 'expand_text ' => __( 'expand child menu', 'twentyseventeen' ), + 'collapse_text' => __( 'collapse child menu', 'twentyseventeen' ), ) ); } @@ -329,34 +349,30 @@ public static function add_nav_menu_styles( $args = array() ) { ob_start(); ?> ', '' ), '', ob_get_clean() ); @@ -384,14 +399,6 @@ public static function add_twentyseventeen_masthead_styles() { }, 11 ); } - /** - * Adjust header height. - * - * @todo Implement. - * @link https://github.com/WordPress/wordpress-develop/blob/a26c24226c6b131a0ed22c722a836c100d3ba254/src/wp-content/themes/twentyseventeen/assets/js/global.js#L88-L103 - */ - public function adjust_header_height() {} - /** * Add styles for the nav menu specifically to deal with AMP running in a no-js context. * From 93c87eb85ea4535912d736e5be83bfd77465ec20 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 3 Jun 2018 22:58:52 -0700 Subject: [PATCH 18/27] Add support for smooth scrolling to content in Twenty Seventeen --- .../class-amp-core-theme-sanitizer.php | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 0f108f93043..0e36eb6f94d 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -49,6 +49,8 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'twentyseventeen' => array( 'dequeue_scripts' => array( 'twentyseventeen-html5', // Only relevant for IE<9. + 'twentyseventeen-global', // There are somethings not yet implemented in AMP. See todos below. + 'jquery-scrollto', // Implemented via add_smooth_scrolling(). 'twentyseventeen-navigation', // Handled by add_nav_menu_styles, add_nav_menu_toggle, add_nav_sub_menu_buttons. 'twentyseventeen-skip-link-focus-fix', // Only needed by IE11 and when admin bar is present. ), @@ -64,7 +66,12 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_nav_menu_styles' => array(), 'add_nav_menu_toggle' => array(), 'add_nav_sub_menu_buttons' => array(), - // @todo Dequeue scripts and replace with AMP functionality where possible. + 'add_smooth_scrolling' => array( + '//header[@id = "masthead"]//a[ contains( @class, "menu-scroll-down" ) ]', + ), + // @todo Implement setQuotesIcon(). + // @todo Add support for sticky nav, that is adjustScrollClass(). + // @todo Try to implement belowEntryMetaClass(). ), 'twentysixteen' => array( 'dequeue_scripts' => array( @@ -263,6 +270,21 @@ public static function remove_actions( $actions = array() ) { } } + /** + * Add smooth scrolling from link to target element. + * + * @param string[] $link_xpaths XPath queries to the links that should smooth scroll. + */ + public function add_smooth_scrolling( $link_xpaths ) { + foreach ( $link_xpaths as $link_xpath ) { + foreach ( $this->xpath->query( $link_xpath ) as $link ) { + if ( $link instanceof DOMElement && preg_match( '/#(.+)/', $link->getAttribute( 'href' ), $matches ) ) { + $link->setAttribute( 'on', sprintf( 'tap:%s.scrollTo(duration=600)', $matches[1] ) ); + } + } + } + } + /** * Force SVG support, replacing no-svg class name with svg class name. * From 7b9acc66e8ae75ea573f5a875b733ece93aa5c63 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 3 Jun 2018 23:47:37 -0700 Subject: [PATCH 19/27] Add accept_validation_errors feature to pre-accept issues in core themes --- .../class-amp-core-theme-sanitizer.php | 81 +++++++++++++++++++ .../class-amp-invalid-url-post-type.php | 2 +- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 0e36eb6f94d..53c29d7a36c 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -69,12 +69,28 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_smooth_scrolling' => array( '//header[@id = "masthead"]//a[ contains( @class, "menu-scroll-down" ) ]', ), + 'accept_validation_errors' => array( + 'removed_unused_css_rules' => true, + 'invalid_element' => array( + array( + 'node_name' => 'meta', + 'parent_name' => 'head', + 'node_attributes' => array( + 'name' => 'viewport', + 'content' => 'width=device-width, initial-scale=1', + ), + ), + ), + ), // @todo Implement setQuotesIcon(). // @todo Add support for sticky nav, that is adjustScrollClass(). // @todo Try to implement belowEntryMetaClass(). ), 'twentysixteen' => array( + // @todo Implement onResizeARIA(). + // @todo Implement belowEntryMetaClass(). 'dequeue_scripts' => array( + 'twentysixteen-script', 'twentysixteen-html5', // Only relevant for IE<9. 'twentysixteen-keyboard-image-navigation', // AMP does not yet allow for listening to keydown events. 'twentysixteen-skip-link-focus-fix', // Only needed by IE11 and when admin bar is present. @@ -87,9 +103,25 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_nav_menu_styles' => array(), 'add_nav_menu_toggle' => array(), 'add_nav_sub_menu_buttons' => array(), + 'accept_validation_errors' => array( + 'removed_unused_css_rules' => true, + 'illegal_css_at_rule' => true, // @todo Be more granular so it only applies in theme's style.css. + 'invalid_element' => array( + array( + 'node_name' => 'meta', + 'parent_name' => 'head', + 'node_attributes' => array( + 'name' => 'viewport', + 'content' => 'width=device-width, initial-scale=1', + ), + ), + ), + ), ), 'twentyfifteen' => array( + // @todo Implement onResizeARIA(). 'dequeue_scripts' => array( + 'twentyfifteen-script', 'twentyfifteen-keyboard-image-navigation', // AMP does not yet allow for listening to keydown events. 'twentyfifteen-skip-link-focus-fix', // Only needed by IE11 and when admin bar is present. ), @@ -101,6 +133,20 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_nav_menu_styles' => array(), 'add_nav_menu_toggle' => array(), 'add_nav_sub_menu_buttons' => array(), + 'accept_validation_errors' => array( + 'removed_unused_css_rules' => true, + 'illegal_css_at_rule' => true, // @todo Be more granular so it only applies in theme's style.css. + 'invalid_element' => array( + array( + 'node_name' => 'meta', + 'parent_name' => 'head', + 'node_attributes' => array( + 'name' => 'viewport', + 'content' => 'width=device-width', + ), + ), + ), + ), ), ); @@ -302,6 +348,41 @@ public function force_svg_support() { ); } + /** + * Accept validation errors. + * + * @param array $acceptable_errors Validation errors to accept. Either an validation error array or an error code. + */ + public function accept_validation_errors( $acceptable_errors ) { + $normalized_acceptable_errors = array(); + foreach ( $acceptable_errors as $code => $acceptable_error_instances ) { + if ( true === $acceptable_error_instances ) { + $normalized_acceptable_errors[ $code ] = $acceptable_error_instances; + } else { + // @todo Switch to using proper subset detection. + foreach ( $acceptable_error_instances as $acceptable_error_instance ) { + $term_data = AMP_Validation_Error_Taxonomy::prepare_validation_error_taxonomy_term( array_merge( $acceptable_error_instance, compact( 'code' ) ) ); + + $normalized_acceptable_errors[ $code ][] = $term_data['slug']; + } + } + } + + add_filter( 'amp_validation_error_sanitized', function( $sanitized, $error ) use ( $normalized_acceptable_errors ) { + if ( isset( $normalized_acceptable_errors[ $error['code'] ] ) ) { + if ( true === $normalized_acceptable_errors[ $error['code'] ] ) { + return true; + } else { + $term_data = AMP_Validation_Error_Taxonomy::prepare_validation_error_taxonomy_term( array_merge( $error, array( 'code' => $error['code'] ) ) ); + if ( in_array( $term_data['slug'], $normalized_acceptable_errors[ $error['code'] ], true ) ) { + return true; + } + } + } + return $sanitized; + }, 10, 2 ); + } + /** * Force support for fixed background-attachment. * diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 8504a36ffce..aa9e9b71f1d 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -1192,7 +1192,7 @@ public static function print_url_as_title( $post ) { * @return string Title. */ public static function filter_the_title_in_post_list_table( $title, $post ) { - if ( get_current_screen()->base === 'edit' && get_current_screen()->post_type === self::POST_TYPE_SLUG && self::POST_TYPE_SLUG === get_post_type( $post ) ) { + if ( function_exists( 'get_current_screen' ) && get_current_screen()->base === 'edit' && get_current_screen()->post_type === self::POST_TYPE_SLUG && self::POST_TYPE_SLUG === get_post_type( $post ) ) { $title = preg_replace( '#^(\w+:)?//[^/]+#', '', $title ); } return $title; From df2e5618b7b2c4c12345159df4f94c334c278b2c Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 4 Jun 2018 16:30:07 +0200 Subject: [PATCH 20/27] Support setQuotesIcon() --- .../class-amp-core-theme-sanitizer.php | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 53c29d7a36c..8ff125624c8 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -82,7 +82,8 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { ), ), ), - // @todo Implement setQuotesIcon(). + 'set_quotes_icon' => array(), + 'add_quotes_icon' => array(), // @todo Add support for sticky nav, that is adjustScrollClass(). // @todo Try to implement belowEntryMetaClass(). ), @@ -205,6 +206,53 @@ public static function add_buffering_hooks( $args = array() ) { } } + /** + * Add filter to output the quote icons in front of the article content. + * + * @since 1.0 + * + * @param array $args Args. + */ + public static function add_quotes_icon( $args = array() ) { + add_filter( 'the_content', function ( $content ) { + $icon = twentyseventeen_get_svg( array( 'icon' => 'quote-right' ) ); + $count = substr_count( $content, 'xpath->query( $args['blockquote_article'] ); + if ( ! $blockquote_el ) { + return; + } + + $svg_el = $this->xpath->query( $args['set_quotes_icon'] ); + if ( 0 === $svg_el->length ) { + return; + } + + foreach ( $blockquote_el as $index => $el ) { + $icon = $svg_el->item( $index ); + $el->insertBefore( $icon, $el->firstChild ); + } + } + /** * Fix up core themes to do things in the AMP way. * @@ -281,6 +329,8 @@ protected static function get_theme_config( $theme ) { 'sub_menu_button_toggle_class' => 'toggled-on', 'expand_text ' => __( 'expand child menu', 'twentyseventeen' ), 'collapse_text' => __( 'collapse child menu', 'twentyseventeen' ), + 'blockquote_article' => '//article[ contains( @class, "format-quote" ) ]//blockquote', + 'set_quotes_icon' => '//article[ contains( @class, "format-quote" ) ]//svg[ contains( @class, "icon-quote-right" ) ]', ) ); } From bca1ead9dc144955b8a2a01b45180956c56f6550 Mon Sep 17 00:00:00 2001 From: David Cramer Date: Mon, 4 Jun 2018 16:42:12 +0200 Subject: [PATCH 21/27] check post format for quote, rename methods --- .../sanitizers/class-amp-core-theme-sanitizer.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 8ff125624c8..49069607e71 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -82,7 +82,7 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { ), ), ), - 'set_quotes_icon' => array(), + 'move_quotes_icon' => array(), 'add_quotes_icon' => array(), // @todo Add support for sticky nav, that is adjustScrollClass(). // @todo Try to implement belowEntryMetaClass(). @@ -215,11 +215,15 @@ public static function add_buffering_hooks( $args = array() ) { */ public static function add_quotes_icon( $args = array() ) { add_filter( 'the_content', function ( $content ) { - $icon = twentyseventeen_get_svg( array( 'icon' => 'quote-right' ) ); - $count = substr_count( $content, ' 'quote-right' ) ); + $count = substr_count( $content, ' Date: Mon, 4 Jun 2018 08:33:44 -0700 Subject: [PATCH 22/27] Simplify implementation of Twenty Seventeen's setQuotesIcon() Also move the get_theme_config method to be closer to logically-related $theme_features class var --- .../class-amp-core-theme-sanitizer.php | 171 +++++++----------- 1 file changed, 69 insertions(+), 102 deletions(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 49069607e71..e953c133288 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -82,8 +82,7 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { ), ), ), - 'move_quotes_icon' => array(), - 'add_quotes_icon' => array(), + 'set_twentyseventeen_quotes_icon' => array(), // @todo Add support for sticky nav, that is adjustScrollClass(). // @todo Try to implement belowEntryMetaClass(). ), @@ -151,6 +150,67 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { ), ); + /** + * Get theme config. + * + * @param string $theme Theme slug. + * @return array Class names. + */ + protected static function get_theme_config( $theme ) { + // phpcs:disable WordPress.WP.I18n.TextDomainMismatch + $config = array( + 'sub_menu_button_class' => 'dropdown-toggle', + ); + switch ( $theme ) { + case 'twentyfifteen': + return array_merge( + $config, + array( + 'nav_container_id' => 'secondary', + 'nav_container_toggle_class' => 'toggled-on', + 'menu_button_class' => 'secondary-toggle', + 'menu_button_xpath' => '//header[ @id = "masthead" ]//button[ contains( @class, "secondary-toggle" ) ]', + 'menu_button_toggle_class' => 'toggled-on', + 'sub_menu_button_toggle_class' => 'toggle-on', + 'expand_text ' => __( 'expand child menu', 'twentyfifteen' ), + 'collapse_text' => __( 'collapse child menu', 'twentyfifteen' ), + ) + ); + + case 'twentysixteen': + return array_merge( + $config, + array( + 'nav_container_id' => 'site-header-menu', + 'nav_container_toggle_class' => 'toggled-on', + 'menu_button_class' => 'menu-toggle', + 'menu_button_xpath' => '//header[@id = "masthead"]//button[ @id = "menu-toggle" ]', + 'menu_button_toggle_class' => 'toggled-on', + 'sub_menu_button_toggle_class' => 'toggled-on', + 'expand_text ' => __( 'expand child menu', 'twentysixteen' ), + 'collapse_text' => __( 'collapse child menu', 'twentysixteen' ), + ) + ); + + case 'twentyseventeen': + default: + return array_merge( + $config, + array( + 'nav_container_id' => 'site-navigation', + 'nav_container_toggle_class' => 'toggled-on', + 'menu_button_class' => 'menu-toggle', + 'menu_button_xpath' => '//nav[@id = "site-navigation"]//button[ contains( @class, "menu-toggle" ) ]', + 'menu_button_toggle_class' => 'toggled-on', + 'sub_menu_button_toggle_class' => 'toggled-on', + 'expand_text ' => __( 'expand child menu', 'twentyseventeen' ), + 'collapse_text' => __( 'collapse child menu', 'twentyseventeen' ), + ) + ); + } + // phpcs:enable WordPress.WP.I18n.TextDomainMismatch + } + /** * Find theme features for core theme. * @@ -209,54 +269,24 @@ public static function add_buffering_hooks( $args = array() ) { /** * Add filter to output the quote icons in front of the article content. * - * @since 1.0 + * This is only used in Twenty Seventeen. * - * @param array $args Args. + * @since 1.0 + * @link https://github.com/WordPress/wordpress-develop/blob/f4580c122b7d0d2d66d22f806c6fe6e11023c6f0/src/wp-content/themes/twentyseventeen/assets/js/global.js#L105-L108 */ - public static function add_quotes_icon( $args = array() ) { + public static function set_twentyseventeen_quotes_icon() { add_filter( 'the_content', function ( $content ) { - if ( 'quote' === get_post_format() ) { + // Why isn't Twenty Seventeen doing this to begin with? Why is it using JS to add the quote icon? + if ( function_exists( 'twentyseventeen_get_svg' ) && 'quote' === get_post_format() ) { $icon = twentyseventeen_get_svg( array( 'icon' => 'quote-right' ) ); - $count = substr_count( $content, ')#s', '$1' . $icon, $content ); } return $content; } ); } - /** - * Move any found quote icons and move them into blockquotes. - * - * @since 1.0 - * - * @param array $args Args. - */ - public function move_quotes_icon( $args = array() ) { - - $args = array_merge( - self::get_theme_config( get_template() ), - $args - ); - - $blockquote_el = $this->xpath->query( $args['blockquote_article'] ); - if ( ! $blockquote_el ) { - return; - } - - $svg_el = $this->xpath->query( $args['set_quotes_icon'] ); - if ( 0 === $svg_el->length ) { - return; - } - - foreach ( $blockquote_el as $index => $el ) { - $icon = $svg_el->item( $index ); - $el->insertBefore( $icon, $el->firstChild ); - } - } - /** * Fix up core themes to do things in the AMP way. * @@ -278,69 +308,6 @@ public function sanitize() { } } - /** - * Get theme config. - * - * @param string $theme Theme slug. - * @return array Class names. - */ - protected static function get_theme_config( $theme ) { - // phpcs:disable WordPress.WP.I18n.TextDomainMismatch - $config = array( - 'sub_menu_button_class' => 'dropdown-toggle', - ); - switch ( $theme ) { - case 'twentyfifteen': - return array_merge( - $config, - array( - 'nav_container_id' => 'secondary', - 'nav_container_toggle_class' => 'toggled-on', - 'menu_button_class' => 'secondary-toggle', - 'menu_button_xpath' => '//header[ @id = "masthead" ]//button[ contains( @class, "secondary-toggle" ) ]', - 'menu_button_toggle_class' => 'toggled-on', - 'sub_menu_button_toggle_class' => 'toggle-on', - 'expand_text ' => __( 'expand child menu', 'twentyfifteen' ), - 'collapse_text' => __( 'collapse child menu', 'twentyfifteen' ), - ) - ); - - case 'twentysixteen': - return array_merge( - $config, - array( - 'nav_container_id' => 'site-header-menu', - 'nav_container_toggle_class' => 'toggled-on', - 'menu_button_class' => 'menu-toggle', - 'menu_button_xpath' => '//header[@id = "masthead"]//button[ @id = "menu-toggle" ]', - 'menu_button_toggle_class' => 'toggled-on', - 'sub_menu_button_toggle_class' => 'toggled-on', - 'expand_text ' => __( 'expand child menu', 'twentysixteen' ), - 'collapse_text' => __( 'collapse child menu', 'twentysixteen' ), - ) - ); - - case 'twentyseventeen': - default: - return array_merge( - $config, - array( - 'nav_container_id' => 'site-navigation', - 'nav_container_toggle_class' => 'toggled-on', - 'menu_button_class' => 'menu-toggle', - 'menu_button_xpath' => '//nav[@id = "site-navigation"]//button[ contains( @class, "menu-toggle" ) ]', - 'menu_button_toggle_class' => 'toggled-on', - 'sub_menu_button_toggle_class' => 'toggled-on', - 'expand_text ' => __( 'expand child menu', 'twentyseventeen' ), - 'collapse_text' => __( 'collapse child menu', 'twentyseventeen' ), - 'blockquote_article' => '//article[ contains( @class, "format-quote" ) ]//blockquote', - 'set_quotes_icon' => '//article[ contains( @class, "format-quote" ) ]//svg[ contains( @class, "icon-quote-right" ) ]', - ) - ); - } - // phpcs:enable WordPress.WP.I18n.TextDomainMismatch - } - /** * Dequeue scripts. * From d55cf6a22314c81ca653e984511c70411265f3ad Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 4 Jun 2018 09:17:32 -0700 Subject: [PATCH 23/27] Implement is_array_subset to have more fine-grained control over whether validation error is accepted --- .../class-amp-core-theme-sanitizer.php | 99 +++++++++++++------ 1 file changed, 70 insertions(+), 29 deletions(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index e953c133288..f05b1bc9782 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -46,7 +46,10 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { * @var array */ protected static $theme_features = array( + // Twenty Seventeen. 'twentyseventeen' => array( + // @todo Add support for sticky nav, that is adjustScrollClass(). + // @todo Try to implement belowEntryMetaClass(). 'dequeue_scripts' => array( 'twentyseventeen-html5', // Only relevant for IE<9. 'twentyseventeen-global', // There are somethings not yet implemented in AMP. See todos below. @@ -83,12 +86,12 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { ), ), 'set_twentyseventeen_quotes_icon' => array(), - // @todo Add support for sticky nav, that is adjustScrollClass(). - // @todo Try to implement belowEntryMetaClass(). ), + + // Twenty Sixteen. 'twentysixteen' => array( - // @todo Implement onResizeARIA(). - // @todo Implement belowEntryMetaClass(). + // @todo Figure out an AMP solution for onResizeARIA(). + // @todo Try to implement belowEntryMetaClass(). 'dequeue_scripts' => array( 'twentysixteen-script', 'twentysixteen-html5', // Only relevant for IE<9. @@ -105,7 +108,20 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_nav_sub_menu_buttons' => array(), 'accept_validation_errors' => array( 'removed_unused_css_rules' => true, - 'illegal_css_at_rule' => true, // @todo Be more granular so it only applies in theme's style.css. + 'illegal_css_at_rule' => array( + array( + 'at_rule' => 'viewport', + 'node_attributes' => array( + 'id' => 'twentysixteen-style-css', + ), + ), + array( + 'at_rule' => '-ms-viewport', + 'node_attributes' => array( + 'id' => 'twentysixteen-style-css', + ), + ), + ), 'invalid_element' => array( array( 'node_name' => 'meta', @@ -118,8 +134,10 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { ), ), ), + + // Twenty Fifteen. 'twentyfifteen' => array( - // @todo Implement onResizeARIA(). + // @todo Figure out an AMP solution for onResizeARIA(). 'dequeue_scripts' => array( 'twentyfifteen-script', 'twentyfifteen-keyboard-image-navigation', // AMP does not yet allow for listening to keydown events. @@ -135,7 +153,20 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_nav_sub_menu_buttons' => array(), 'accept_validation_errors' => array( 'removed_unused_css_rules' => true, - 'illegal_css_at_rule' => true, // @todo Be more granular so it only applies in theme's style.css. + 'illegal_css_at_rule' => array( + array( + 'at_rule' => 'viewport', + 'node_attributes' => array( + 'id' => 'twentyfifteen-style-css', + ), + ), + array( + 'at_rule' => '-ms-viewport', + 'node_attributes' => array( + 'id' => 'twentyfifteen-style-css', + ), + ), + ), 'invalid_element' => array( array( 'node_name' => 'meta', @@ -372,30 +403,17 @@ public function force_svg_support() { /** * Accept validation errors. * + * @todo This needs to be called even in an admin context!!! * @param array $acceptable_errors Validation errors to accept. Either an validation error array or an error code. */ - public function accept_validation_errors( $acceptable_errors ) { - $normalized_acceptable_errors = array(); - foreach ( $acceptable_errors as $code => $acceptable_error_instances ) { - if ( true === $acceptable_error_instances ) { - $normalized_acceptable_errors[ $code ] = $acceptable_error_instances; - } else { - // @todo Switch to using proper subset detection. - foreach ( $acceptable_error_instances as $acceptable_error_instance ) { - $term_data = AMP_Validation_Error_Taxonomy::prepare_validation_error_taxonomy_term( array_merge( $acceptable_error_instance, compact( 'code' ) ) ); - - $normalized_acceptable_errors[ $code ][] = $term_data['slug']; - } - } - } - - add_filter( 'amp_validation_error_sanitized', function( $sanitized, $error ) use ( $normalized_acceptable_errors ) { - if ( isset( $normalized_acceptable_errors[ $error['code'] ] ) ) { - if ( true === $normalized_acceptable_errors[ $error['code'] ] ) { + public static function accept_validation_errors( $acceptable_errors ) { + add_filter( 'amp_validation_error_sanitized', function( $sanitized, $error ) use ( $acceptable_errors ) { + if ( isset( $acceptable_errors[ $error['code'] ] ) ) { + if ( true === $acceptable_errors[ $error['code'] ] ) { return true; - } else { - $term_data = AMP_Validation_Error_Taxonomy::prepare_validation_error_taxonomy_term( array_merge( $error, array( 'code' => $error['code'] ) ) ); - if ( in_array( $term_data['slug'], $normalized_acceptable_errors[ $error['code'] ], true ) ) { + } + foreach ( $acceptable_errors[ $error['code'] ] as $acceptable_error_props ) { + if ( AMP_Core_Theme_Sanitizer::is_array_subset( $error, $acceptable_error_props ) ) { return true; } } @@ -404,6 +422,30 @@ public function accept_validation_errors( $acceptable_errors ) { }, 10, 2 ); } + /** + * Check if one array is a sparse subset of another array. + * + * @param array $superset Superset array. + * @param array $subset Subset array. + * + * @return bool Whether subset is contained in superset. + */ + public static function is_array_subset( $superset, $subset ) { + foreach ( $subset as $key => $subset_value ) { + if ( ! isset( $superset[ $key ] ) || gettype( $subset_value ) !== gettype( $superset[ $key ] ) ) { + return false; + } + if ( is_array( $subset_value ) ) { + if ( ! self::is_array_subset( $superset[ $key ], $subset_value ) ) { + return false; + } + } elseif ( $superset[ $key ] !== $subset_value ) { + return false; + } + } + return true; + } + /** * Force support for fixed background-attachment. * @@ -692,7 +734,6 @@ public function add_nav_menu_toggle( $args = array() ) { $state_id = 'navMenuToggledOn'; $expanded = false; - // @todo Not twentyfifteen? $nav_el->setAttribute( AMP_DOM_Utils::get_amp_bind_placeholder_prefix() . 'class', sprintf( From 425c80a85a8690ac1187379d8f78ab998a425825 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 4 Jun 2018 11:24:50 -0700 Subject: [PATCH 24/27] Accept core theme validation errors in admin as well as frontend --- includes/amp-helper-functions.php | 4 + includes/class-amp-theme-support.php | 8 - .../class-amp-core-theme-sanitizer.php | 188 ++++++++---------- .../class-amp-validation-error-taxonomy.php | 54 +++++ 4 files changed, 137 insertions(+), 117 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 066cf60f7d8..10002c56e48 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -480,6 +480,10 @@ function amp_get_content_sanitizers( $post = null ) { */ $sanitizers = apply_filters( 'amp_content_sanitizers', array( + 'AMP_Core_Theme_Sanitizer' => array( + 'template' => get_template(), + 'stylesheet' => get_stylesheet(), + ), 'AMP_Img_Sanitizer' => array(), 'AMP_Form_Sanitizer' => array(), 'AMP_Comments_Sanitizer' => array(), diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index c29f864d5b6..5539593f130 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -256,14 +256,6 @@ public static function register_paired_hooks() { */ public static function add_hooks() { - add_filter( 'amp_content_sanitizers', function( $sanitizers ) { - $sanitizers['AMP_Core_Theme_Sanitizer'] = array( - 'template' => get_template(), - 'stylesheet' => get_stylesheet(), - ); - return $sanitizers; - } ); - // Remove core actions which are invalid AMP. remove_action( 'wp_head', 'wp_post_preview_js', 1 ); remove_action( 'wp_head', 'print_emoji_detection_script', 7 ); diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index f05b1bc9782..3696a7147bc 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -11,6 +11,7 @@ * * Fixes up common issues in core themes and others. * + * @see AMP_Validation_Error_Taxonomy::accept_core_theme_validation_errors() * @since 1.0 */ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { @@ -72,19 +73,6 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_smooth_scrolling' => array( '//header[@id = "masthead"]//a[ contains( @class, "menu-scroll-down" ) ]', ), - 'accept_validation_errors' => array( - 'removed_unused_css_rules' => true, - 'invalid_element' => array( - array( - 'node_name' => 'meta', - 'parent_name' => 'head', - 'node_attributes' => array( - 'name' => 'viewport', - 'content' => 'width=device-width, initial-scale=1', - ), - ), - ), - ), 'set_twentyseventeen_quotes_icon' => array(), ), @@ -106,33 +94,6 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_nav_menu_styles' => array(), 'add_nav_menu_toggle' => array(), 'add_nav_sub_menu_buttons' => array(), - 'accept_validation_errors' => array( - 'removed_unused_css_rules' => true, - 'illegal_css_at_rule' => array( - array( - 'at_rule' => 'viewport', - 'node_attributes' => array( - 'id' => 'twentysixteen-style-css', - ), - ), - array( - 'at_rule' => '-ms-viewport', - 'node_attributes' => array( - 'id' => 'twentysixteen-style-css', - ), - ), - ), - 'invalid_element' => array( - array( - 'node_name' => 'meta', - 'parent_name' => 'head', - 'node_attributes' => array( - 'name' => 'viewport', - 'content' => 'width=device-width, initial-scale=1', - ), - ), - ), - ), ), // Twenty Fifteen. @@ -151,35 +112,90 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'add_nav_menu_styles' => array(), 'add_nav_menu_toggle' => array(), 'add_nav_sub_menu_buttons' => array(), - 'accept_validation_errors' => array( - 'removed_unused_css_rules' => true, - 'illegal_css_at_rule' => array( - array( - 'at_rule' => 'viewport', - 'node_attributes' => array( - 'id' => 'twentyfifteen-style-css', + ), + ); + + /** + * Get the acceptable validation errors. + * + * @param string $template Template. + * @return array Acceptable errors. + */ + public static function get_acceptable_errors( $template ) { + switch ( $template ) { + case 'twentyfifteen': + return array( + 'removed_unused_css_rules' => true, + 'illegal_css_at_rule' => array( + array( + 'at_rule' => 'viewport', + 'node_attributes' => array( + 'id' => 'twentyfifteen-style-css', + ), + ), + array( + 'at_rule' => '-ms-viewport', + 'node_attributes' => array( + 'id' => 'twentyfifteen-style-css', + ), ), ), - array( - 'at_rule' => '-ms-viewport', - 'node_attributes' => array( - 'id' => 'twentyfifteen-style-css', + 'invalid_element' => array( + array( + 'node_name' => 'meta', + 'parent_name' => 'head', + 'node_attributes' => array( + 'name' => 'viewport', + 'content' => 'width=device-width', + ), ), ), - ), - 'invalid_element' => array( - array( - 'node_name' => 'meta', - 'parent_name' => 'head', - 'node_attributes' => array( - 'name' => 'viewport', - 'content' => 'width=device-width', + ); + case 'twentysixteen': + return array( + 'removed_unused_css_rules' => true, + 'illegal_css_at_rule' => array( + array( + 'at_rule' => 'viewport', + 'node_attributes' => array( + 'id' => 'twentysixteen-style-css', + ), + ), + array( + 'at_rule' => '-ms-viewport', + 'node_attributes' => array( + 'id' => 'twentysixteen-style-css', + ), ), ), - ), - ), - ), - ); + 'invalid_element' => array( + array( + 'node_name' => 'meta', + 'parent_name' => 'head', + 'node_attributes' => array( + 'name' => 'viewport', + 'content' => 'width=device-width, initial-scale=1', + ), + ), + ), + ); + case 'twentyseventeen': + return array( + 'removed_unused_css_rules' => true, + 'invalid_element' => array( + array( + 'node_name' => 'meta', + 'parent_name' => 'head', + 'node_attributes' => array( + 'name' => 'viewport', + 'content' => 'width=device-width, initial-scale=1', + ), + ), + ), + ); + } + return array(); + } /** * Get theme config. @@ -400,52 +416,6 @@ public function force_svg_support() { ); } - /** - * Accept validation errors. - * - * @todo This needs to be called even in an admin context!!! - * @param array $acceptable_errors Validation errors to accept. Either an validation error array or an error code. - */ - public static function accept_validation_errors( $acceptable_errors ) { - add_filter( 'amp_validation_error_sanitized', function( $sanitized, $error ) use ( $acceptable_errors ) { - if ( isset( $acceptable_errors[ $error['code'] ] ) ) { - if ( true === $acceptable_errors[ $error['code'] ] ) { - return true; - } - foreach ( $acceptable_errors[ $error['code'] ] as $acceptable_error_props ) { - if ( AMP_Core_Theme_Sanitizer::is_array_subset( $error, $acceptable_error_props ) ) { - return true; - } - } - } - return $sanitized; - }, 10, 2 ); - } - - /** - * Check if one array is a sparse subset of another array. - * - * @param array $superset Superset array. - * @param array $subset Subset array. - * - * @return bool Whether subset is contained in superset. - */ - public static function is_array_subset( $superset, $subset ) { - foreach ( $subset as $key => $subset_value ) { - if ( ! isset( $superset[ $key ] ) || gettype( $subset_value ) !== gettype( $superset[ $key ] ) ) { - return false; - } - if ( is_array( $subset_value ) ) { - if ( ! self::is_array_subset( $superset[ $key ], $subset_value ) ) { - return false; - } - } elseif ( $superset[ $key ] !== $subset_value ) { - return false; - } - } - return true; - } - /** * Force support for fixed background-attachment. * diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 381e415ca4d..e63c36ee148 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -158,6 +158,8 @@ public static function register() { if ( is_admin() ) { self::add_admin_hooks(); } + + self::accept_validation_errors( AMP_Core_Theme_Sanitizer::get_acceptable_errors( get_template() ) ); } /** @@ -246,6 +248,58 @@ public static function get_validation_error_sanitization( $error ) { return compact( 'status', 'forced' ); } + /** + * Automatically (forcibly) accept validation errors that arise. + * + * @since 1.0 + * @see AMP_Core_Theme_Sanitizer::get_acceptable_errors() + * + * @param array $acceptable_errors Acceptable validation errors, where keys are codes and values are either `true` or sparse array to check as subset. + */ + public static function accept_validation_errors( $acceptable_errors ) { + if ( empty( $acceptable_errors ) ) { + return; + } + + /** + * Check if one array is a sparse subset of another array. + * + * @param array $superset Superset array. + * @param array $subset Subset array. + * + * @return bool Whether subset is contained in superset. + */ + $is_array_subset = function( $superset, $subset ) use ( &$is_array_subset ) { + foreach ( $subset as $key => $subset_value ) { + if ( ! isset( $superset[ $key ] ) || gettype( $subset_value ) !== gettype( $superset[ $key ] ) ) { + return false; + } + if ( is_array( $subset_value ) ) { + if ( ! $is_array_subset( $superset[ $key ], $subset_value ) ) { + return false; + } + } elseif ( $superset[ $key ] !== $subset_value ) { + return false; + } + } + return true; + }; + + add_filter( 'amp_validation_error_sanitized', function( $sanitized, $error ) use ( $is_array_subset, $acceptable_errors ) { + if ( isset( $acceptable_errors[ $error['code'] ] ) ) { + if ( true === $acceptable_errors[ $error['code'] ] ) { + return true; + } + foreach ( $acceptable_errors[ $error['code'] ] as $acceptable_error_props ) { + if ( $is_array_subset( $error, $acceptable_error_props ) ) { + return true; + } + } + } + return $sanitized; + }, 10, 2 ); + } + /** * Get the count of validation error terms, optionally restricted by term group (e.g. accepted or rejected). * From 70d14404fb9f9af319226fab04a65a9a6a6569b3 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 4 Jun 2018 13:38:10 -0700 Subject: [PATCH 25/27] Implement sticky header for Twenty Seventeen --- .../class-amp-core-theme-sanitizer.php | 133 ++++++++++++++++-- 1 file changed, 124 insertions(+), 9 deletions(-) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 3696a7147bc..1eaa2bd5f49 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -19,6 +19,7 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { /** * Array of flags used to control sanitization. * + * @since 1.0 * @var array { * @type string $stylesheet Stylesheet slug. * @type string $template Template slug. @@ -30,6 +31,7 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { /** * Body element. * + * @since 1.0 * @var DOMElement */ protected $body; @@ -37,6 +39,7 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { /** * XPath. * + * @since 1.0 * @var DOMXPath */ protected $xpath; @@ -44,12 +47,12 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { /** * Config for features needed by themes. * + * @since 1.0 * @var array */ protected static $theme_features = array( // Twenty Seventeen. 'twentyseventeen' => array( - // @todo Add support for sticky nav, that is adjustScrollClass(). // @todo Try to implement belowEntryMetaClass(). 'dequeue_scripts' => array( 'twentyseventeen-html5', // Only relevant for IE<9. @@ -66,6 +69,7 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { 'force_svg_support' => array(), 'force_fixed_background_support' => array(), 'add_twentyseventeen_masthead_styles' => array(), + 'add_twentyseventeen_sticky_nav_menu' => array(), 'add_has_header_video_body_class' => array(), 'add_nav_menu_styles' => array(), 'add_nav_menu_toggle' => array(), @@ -118,6 +122,8 @@ class AMP_Core_Theme_Sanitizer extends AMP_Base_Sanitizer { /** * Get the acceptable validation errors. * + * @since 1.0 + * * @param string $template Template. * @return array Acceptable errors. */ @@ -200,6 +206,8 @@ public static function get_acceptable_errors( $template ) { /** * Get theme config. * + * @since 1.0 + * * @param string $theme Theme slug. * @return array Class names. */ @@ -261,6 +269,8 @@ protected static function get_theme_config( $theme ) { /** * Find theme features for core theme. * + * @since 1.0 + * * @param array $args Args. * @param bool $static Static. that is, whether should run during output buffering. * @return array Theme features. @@ -358,6 +368,8 @@ public function sanitize() { /** * Dequeue scripts. * + * @since 1.0 + * * @param string[] $handles Handles, where each item value is the script handle. */ public static function dequeue_scripts( $handles = array() ) { @@ -371,6 +383,8 @@ public static function dequeue_scripts( $handles = array() ) { /** * Remove actions. * + * @since 1.0 + * * @param array $actions Actions, with action name as key and value being callback. */ public static function remove_actions( $actions = array() ) { @@ -387,6 +401,8 @@ public static function remove_actions( $actions = array() ) { /** * Add smooth scrolling from link to target element. * + * @since 1.0 + * * @param string[] $link_xpaths XPath queries to the links that should smooth scroll. */ public function add_smooth_scrolling( $link_xpaths ) { @@ -402,6 +418,8 @@ public function add_smooth_scrolling( $link_xpaths ) { /** * Force SVG support, replacing no-svg class name with svg class name. * + * @since 1.0 + * * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/assets/js/global.js#L211-L213 * @link https://caniuse.com/#feat=svg */ @@ -419,6 +437,8 @@ public function force_svg_support() { /** * Force support for fixed background-attachment. * + * @since 1.0 + * * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/assets/js/global.js#L215-L217 * @link https://caniuse.com/#feat=background-attachment */ @@ -432,6 +452,7 @@ public function force_fixed_background_support() { /** * Add body class when there is a header video. * + * @since 1.0 * @link https://github.com/WordPress/wordpress-develop/blob/a26c24226c6b131a0ed22c722a836c100d3ba254/src/wp-content/themes/twentyseventeen/assets/js/global.js#L244-L247 * * @param array $args Args. @@ -457,6 +478,7 @@ public static function add_has_header_video_body_class( $args = array() ) { * * This is currently used exclusively for Twenty Seventeen. * + * @since 1.0 * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1687 * @link https://github.com/WordPress/wordpress-develop/blob/1af1f65a21a1a697fb5f33027497f9e5ae638453/src/wp-content/themes/twentyseventeen/style.css#L1743 */ @@ -469,6 +491,7 @@ public static function add_twentyseventeen_masthead_styles() { * Nevertheless, object-fit does not apply on amp-img and it needs to apply on an actual img. */ add_action( 'wp_enqueue_scripts', function() use ( $args ) { + $is_front_page_layout = ( is_front_page() && 'posts' !== get_option( 'show_on_front' ) ) || ( is_home() && is_front_page() ); ob_start(); ?> xpath->query( '//header[ @id = "masthead" ]//div[ contains( @class, "navigation-top" ) ]' )->item( 0 ); + if ( ! $navigation_top ) { + return; + } + + $navigation_top_fixed = $navigation_top->cloneNode( true ); + $navigation_top_fixed->setAttribute( 'class', $navigation_top_fixed->getAttribute( 'class' ) . ' site-navigation-fixed' ); + + $navigation_top_fixed->setAttribute( 'aria-hidden', 'true' ); + foreach ( $navigation_top_fixed->getElementsByTagName( 'a' ) as $link ) { + $link->setAttribute( 'tabindex', '-1' ); + } + + $navigation_top->parentNode->insertBefore( $navigation_top_fixed, $navigation_top->nextSibling ); + + $position_observer = AMP_DOM_Utils::create_node( $this->dom, 'amp-position-observer', array( + 'layout' => 'nodisplay', + 'intersection-ratios' => 1, + 'on' => implode( ';', array( + 'exit:navigationTopShow.start', + 'enter:navigationTopHide.start', + ) ), + ) ); + $navigation_top->appendChild( $position_observer ); + + $animations = array( + 'navigationTopShow' => array( + 'duration' => 0, + 'fill' => 'both', + 'animations' => array( + 'selector' => '.navigation-top.site-navigation-fixed', + 'media' => '(min-width: 48em)', + 'keyframes' => array( + 'opacity' => 1.0, + 'transform' => 'translateY( 0 )', + ), + ), + ), + 'navigationTopHide' => array( + 'duration' => 0, + 'fill' => 'both', + 'animations' => array( + 'selector' => '.navigation-top.site-navigation-fixed', + 'media' => '(min-width: 48em)', + 'keyframes' => array( + 'opacity' => 0.0, + 'transform' => 'translateY( -72px )', + ), + ), + ), + ); + + foreach ( $animations as $animation_id => $animation ) { + $amp_animation = AMP_DOM_Utils::create_node( $this->dom, 'amp-animation', array( + 'id' => $animation_id, + 'layout' => 'nodisplay', + ) ); + $position_script = $this->dom->createElement( 'script' ); + $position_script->setAttribute( 'type', 'application/json' ); + $position_script->appendChild( $this->dom->createTextNode( wp_json_encode( $animation ) ) ); + $amp_animation->appendChild( $position_script ); + $this->body->appendChild( $amp_animation ); + } + } + /** * Add styles for the nav menu specifically to deal with AMP running in a no-js context. * + * @since 1.0 + * * @param array $args Args. */ public static function add_nav_menu_styles( $args = array() ) { @@ -683,6 +795,8 @@ public static function add_nav_menu_styles( $args = array() ) { /** * Ensure that JS-only nav menu styles apply to AMP as well since even though scripts are not allowed, there are AMP-bind implementations. * + * @since 1.0 + * * @param array $args Args. */ public function add_nav_menu_toggle( $args = array() ) { @@ -734,6 +848,7 @@ public function add_nav_menu_toggle( $args = array() ) { /** * Add buttons for nav sub-menu items. * + * @since 1.0 * @link https://github.com/WordPress/wordpress-develop/blob/a26c24226c6b131a0ed22c722a836c100d3ba254/src/wp-content/themes/twentyseventeen/assets/js/navigation.js#L11-L43 * * @param array $args Args. From 1fdb3257bb4f12f50c2fb2ae6a9499d7999c8c76 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 4 Jun 2018 13:59:10 -0700 Subject: [PATCH 26/27] Prevent showing cloned sticky nav menu on mobile --- includes/sanitizers/class-amp-core-theme-sanitizer.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php index 1eaa2bd5f49..b07854bb1a7 100644 --- a/includes/sanitizers/class-amp-core-theme-sanitizer.php +++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php @@ -539,6 +539,10 @@ public static function add_twentyseventeen_masthead_styles() { } } + .navigation-top.site-navigation-fixed { + display: none; + } + @media screen and (min-width: 48em) { /* Note that adjustHeaderHeight() is irrelevant with this change */ @@ -551,6 +555,7 @@ public static function add_twentyseventeen_masthead_styles() { .navigation-top.site-navigation-fixed { opacity: 0; transform: translateY( -72px ); + display: block; } } From 2fc015d29d03edfd31a59b522b9e4612e4d1b89f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 4 Jun 2018 16:34:18 -0700 Subject: [PATCH 27/27] Remove unnecessary output_header_image method --- includes/class-amp-theme-support.php | 30 ++++------------------------ 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 5539593f130..5097844e350 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1290,12 +1290,13 @@ public static function enqueue_assets() { * @return string $html Filtered markup. */ public static function conditionally_output_header( $html, $header, $atts ) { + unset( $header ); if ( ! is_header_video_active() ) { return $html; }; if ( ! has_header_video() ) { - return self::output_header_image( $atts ); + return AMP_HTML_Utils::build_tag( 'amp-img', $atts ); } return self::output_header_video( $atts ); @@ -1304,13 +1305,10 @@ public static function conditionally_output_header( $html, $header, $atts ) { /** * Replace the header image markup with a header video. * - * This is JS-driven in Core themes like Twenty Sixteen and Twenty Seventeen. - * So in order for the header video to display, - * this replaces the markup of the header image. - * * @since 1.0 * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L78 + * * @param array $atts The header tag attributes array. * @return string $html Filtered markup. */ @@ -1331,7 +1329,7 @@ public static function output_header_video( $atts ) { ); // Create image banner to stay behind the video. - $image_header = self::output_header_image( $atts ); + $image_header = AMP_HTML_Utils::build_tag( 'amp-img', $atts ); // If the video URL is for YouTube, return an element. if ( isset( $parsed_url['host'], $query['v'] ) && ( false !== strpos( $parsed_url['host'], 'youtube' ) ) ) { @@ -1361,24 +1359,4 @@ public static function output_header_video( $atts ) { return $image_header . $video_header; } - - /** - * Replace the header image markup with a header image. - * - * This is JS-driven in Core themes like Twenty Sixteen and Twenty Seventeen. - * So in order for the header video to display, - * this replaces the markup of the header image. - * - * @since 1.0 - * @link https://github.com/WordPress/wordpress-develop/blob/d002fde80e5e3a083e5f950313163f566561517f/src/wp-includes/js/wp-custom-header.js#L54 - * @param array $atts The image tag attributes. - * @return string $html Filtered markup. - */ - public static function output_header_image( $atts ) { - - $place_holder = AMP_HTML_Utils::build_tag( 'amp-img', $atts ); - - return $place_holder; - - } }