From d59b24ad4ba5b7f5c2bdd76989783196f9b606c5 Mon Sep 17 00:00:00 2001
From: Weston Ruter <westonruter@google.com>
Date: Mon, 23 Mar 2020 15:37:22 -0700
Subject: [PATCH] Fix display of Custom Logo in Twenty Twenty

---
 .../class-amp-core-theme-sanitizer.php        | 60 ++++++++++++++++++-
 .../test-class-amp-core-theme-sanitizer.php   | 26 ++++++++
 2 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/includes/sanitizers/class-amp-core-theme-sanitizer.php b/includes/sanitizers/class-amp-core-theme-sanitizer.php
index 4fa4fe418f4..ebae37373bf 100644
--- a/includes/sanitizers/class-amp-core-theme-sanitizer.php
+++ b/includes/sanitizers/class-amp-core-theme-sanitizer.php
@@ -95,6 +95,7 @@ protected static function get_theme_features_config( $theme_slug ) {
 					'add_nav_menu_styles'              => [],
 					'add_twentytwenty_masthead_styles' => [],
 					'add_img_display_block_fix'        => [],
+					'add_twentytwenty_custom_logo_fix' => [],
 					'add_twentytwenty_current_page_awareness' => [],
 				];
 
@@ -720,6 +721,58 @@ static function() {
 		);
 	}
 
+	/**
+	 * Fix display of Custom Logo in Twenty Twenty.
+	 *
+	 * This is required because width:auto on the site-logo amp-img does not preserve the proportional width in the same
+	 * way as the same styles applied to an img.
+	 *
+	 * @since 1.5
+	 * @link https://github.com/ampproject/amp-wp/issues/4418
+	 * @link https://codepen.io/westonruter/pen/rNVqadv
+	 */
+	public static function add_twentytwenty_custom_logo_fix() {
+		$method = __METHOD__;
+		add_filter(
+			'get_custom_logo',
+			static function( $html ) use ( $method ) {
+				// Pattern sourced from AMP_Base_Embed_Handler::match_element_attributes().
+				$pattern = sprintf(
+					'/<img%s/',
+					implode(
+						'',
+						array_map(
+							function ( $attr_name ) {
+								return sprintf( '(?=[^>]*?%1$s="(?P<%1$s>\d+)")?', preg_quote( $attr_name, '/' ) );
+							},
+							[ 'width', 'height' ]
+						)
+					)
+				);
+				if ( preg_match( $pattern, $html, $matches ) && isset( $matches['width'] ) && isset( $matches['height'] ) ) {
+					$width  = (int) $matches['width'];
+					$height = (int) $matches['height'];
+
+					$desktop_height = 9; // in rem; see <https://github.com/WordPress/wordpress-develop/blob/ad8d01a7e9e13144d1676b8e6d70c3e81ef703af/src/wp-content/themes/twentytwenty/style.css#L4887>.
+					$mobile_height  = 6; // in rem; see <https://github.com/WordPress/wordpress-develop/blob/ad8d01a7e9e13144d1676b8e6d70c3e81ef703af/src/wp-content/themes/twentytwenty/style.css#L1424>.
+
+					$desktop_width = $desktop_height * ( $width / $height );
+					$mobile_width  = $mobile_height * ( $width / $height );
+
+					$html .= sprintf(
+						'<style data-src="%s">.site-logo amp-img { width: %frem; } @media (min-width: 700px) { .site-logo amp-img { width: %frem; } }</style>',
+						esc_attr( $method ),
+						$mobile_width,
+						$desktop_width
+					);
+
+				}
+				return $html;
+			},
+			PHP_INT_MAX
+		);
+	}
+
 	/**
 	 * Add style rule with a selector of higher specificity than just `img` to make `amp-img` have `display:block` rather than `display:inline-block`.
 	 *
@@ -728,15 +781,18 @@ static function() {
 	 * resulting in larger margins in AMP than expected.
 	 *
 	 * @since 1.5
+	 * @link https://github.com/ampproject/amp-wp/issues/4419
 	 */
 	public static function add_img_display_block_fix() {
+		$method = __METHOD__;
 		// Note that wp_add_inline_style() is not used because this stylesheet needs to be added _before_ style.css so
 		// that any subsequent style rules for images will continue to override.
 		add_action(
 			'wp_print_styles',
-			static function() {
+			static function() use ( $method ) {
 				printf(
-					'<style data-src="AMP_Core_Theme_Sanitizer::add_img_display_block_fix">%s</style>',
+					'<style data-src="%s">%s</style>',
+					esc_attr( $method ),
 					// The selector is targeting an attribute that can never appear. It is purely present to increase specificity.
 					'amp-img:not([_]) { display: block }'
 				);
diff --git a/tests/php/test-class-amp-core-theme-sanitizer.php b/tests/php/test-class-amp-core-theme-sanitizer.php
index 4e38719d4b0..a289c3e9e0c 100644
--- a/tests/php/test-class-amp-core-theme-sanitizer.php
+++ b/tests/php/test-class-amp-core-theme-sanitizer.php
@@ -251,4 +251,30 @@ public function test_add_img_display_block_fix() {
 		$output = ob_get_clean();
 		$this->assertRegExp( '/amp-img.+display.+block/s', $output );
 	}
+
+	/**
+	 * Tests add_twentytwenty_custom_logo_fix.
+	 *
+	 * @covers AMP_Core_Theme_Sanitizer::add_twentytwenty_custom_logo_fix()
+	 */
+	public function test_add_twentytwenty_custom_logo_fix() {
+		add_filter(
+			'get_custom_logo',
+			static function () {
+				return '<img src="https://example.com/logo.jpg" width="100" height="200">';
+			}
+		);
+
+		AMP_Core_Theme_Sanitizer::add_twentytwenty_custom_logo_fix();
+		$logo = get_custom_logo();
+
+		$needle = '.site-logo amp-img { width: 3.000000rem; } @media (min-width: 700px) { .site-logo amp-img { width: 4.500000rem; } }';
+
+		// @todo Make use of AssertContainsCompatibility trait.
+		if ( method_exists( $this, 'assertStringContainsString' ) ) {
+			$this->assertStringContainsString( $needle, $logo );
+		} else {
+			$this->assertContains( $needle, $logo );
+		}
+	}
 }