From 7c53868baf9e1fc0a1d31fd8718705befba3d597 Mon Sep 17 00:00:00 2001 From: thelovekesh Date: Wed, 9 Nov 2022 11:47:17 +0530 Subject: [PATCH 1/9] Add guard to check if passed url path is same as home_url --- includes/sanitizers/class-amp-link-sanitizer.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index 18899102336..896c287b035 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -52,6 +52,13 @@ class AMP_Link_Sanitizer extends AMP_Base_Sanitizer { */ protected $home_host; + /** + * Home path. + * + * @var string + */ + protected $home_path; + /** * Content path. * @@ -79,7 +86,9 @@ public function __construct( $dom, array $args = [] ) { parent::__construct( $dom, $args ); - $this->home_host = wp_parse_url( home_url(), PHP_URL_HOST ); + $parsed_home = wp_parse_url( home_url() ); + $this->home_host = $parsed_home['host']; + $this->home_path = $parsed_home['path']; $this->content_path = wp_parse_url( content_url( '/' ), PHP_URL_PATH ); $this->admin_path = wp_parse_url( admin_url(), PHP_URL_PATH ); } @@ -295,6 +304,11 @@ public function is_frontend_url( $url ) { return false; } + // Skip adding query var to links on other paths. + if ( ! empty( $parsed_url['path'] ) && false !== strpos( $parsed_url['path'], $this->home_path ) ) { + return false; + } + // Skip adding query var to PHP files (e.g. wp-login.php). if ( ! empty( $parsed_url['path'] ) && preg_match( '/\.php$/', $parsed_url['path'] ) ) { return false; From 5e1999c45b1a14571ad36665039de9091f18d9d4 Mon Sep 17 00:00:00 2001 From: Lovekesh Kumar Date: Thu, 10 Nov 2022 10:22:39 +0530 Subject: [PATCH 2/9] Update path check to find only first occurance in string Co-authored-by: Weston Ruter --- includes/sanitizers/class-amp-link-sanitizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index 896c287b035..2f7cc0fa9f5 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -305,7 +305,7 @@ public function is_frontend_url( $url ) { } // Skip adding query var to links on other paths. - if ( ! empty( $parsed_url['path'] ) && false !== strpos( $parsed_url['path'], $this->home_path ) ) { + if ( ! empty( $parsed_url['path'] ) && 0 !== strpos( $parsed_url['path'], $this->home_path ) ) { return false; } From d4d843f035ff63f8afee4f9ae2bc25ff62d93f39 Mon Sep 17 00:00:00 2001 From: Lovekesh Kumar Date: Thu, 10 Nov 2022 10:24:28 +0530 Subject: [PATCH 3/9] Add default values in case of null occurances in home path and host Co-authored-by: Weston Ruter --- includes/sanitizers/class-amp-link-sanitizer.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/sanitizers/class-amp-link-sanitizer.php b/includes/sanitizers/class-amp-link-sanitizer.php index 2f7cc0fa9f5..108d481dab5 100644 --- a/includes/sanitizers/class-amp-link-sanitizer.php +++ b/includes/sanitizers/class-amp-link-sanitizer.php @@ -86,9 +86,9 @@ public function __construct( $dom, array $args = [] ) { parent::__construct( $dom, $args ); - $parsed_home = wp_parse_url( home_url() ); - $this->home_host = $parsed_home['host']; - $this->home_path = $parsed_home['path']; + $parsed_home = wp_parse_url( home_url( '/' ) ); + $this->home_host = $parsed_home['host'] ?? null; + $this->home_path = $parsed_home['path'] ?? '/'; $this->content_path = wp_parse_url( content_url( '/' ), PHP_URL_PATH ); $this->admin_path = wp_parse_url( admin_url(), PHP_URL_PATH ); } From 205283437000a494a359bec4a0df7a1f1d6f767a Mon Sep 17 00:00:00 2001 From: thelovekesh Date: Thu, 10 Nov 2022 17:24:22 +0530 Subject: [PATCH 4/9] Add test cases for AMP_Link_Sanitizer::is_frontend_url() --- tests/php/test-class-amp-link-sanitizer.php | 73 +++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/tests/php/test-class-amp-link-sanitizer.php b/tests/php/test-class-amp-link-sanitizer.php index ae48bf1d279..a96ea64ce2a 100644 --- a/tests/php/test-class-amp-link-sanitizer.php +++ b/tests/php/test-class-amp-link-sanitizer.php @@ -322,4 +322,77 @@ public function test_amp_to_amp_linking_enabled( $filter, $expected ) { $this->assertArrayNotHasKey( AMP_Link_Sanitizer::class, $sanitizers ); } } + + /** + * Get data for test_is_frontend_url + * + * @return array + */ + public function get_test_is_frontend_url() { + return [ + 'no_scheme' => [ + '//example.com/', + false, + ], + 'invalid_scheme' => [ + 'ftp://example.com/', + false, + ], + 'different_host' => [ + 'https://example.org/', + false, + ], + 'different_path' => [ + home_url( '/foo' ), + false, + ], + 'php_file' => [ + home_url( '/foo.php' ), + false, + ], + 'feed' => [ + home_url( '/feed/' ), + false, + ], + 'admin' => [ + admin_url(), + false, + ], + 'content' => [ + content_url( '/' ), + false, + ], + 'valid' => [ + home_url( '/' ), + true, + ], + ]; + } + + /** + * Test is_frontend_url. + * + * @dataProvider get_test_is_frontend_url + * @covers AMP_Link_Sanitizer::is_frontend_url() + * + * @param string $url URL. + * @param bool $expected Expected. + */ + public function test_is_frontend_url( $url, $expected ) { + $dom = AMP_DOM_Utils::get_dom_from_content( 'Foo' ); + + if ( home_url( '/foo' ) === $url ) { + $new_home_url = home_url( '/bar/' ); + + add_filter( + 'home_url', + static function() use ( $new_home_url ) { + return $new_home_url; + } + ); + } + + $sanitizer = new AMP_Link_Sanitizer( $dom ); + $this->assertEquals( $expected, $sanitizer->is_frontend_url( $url ) ); + } } From d4f6d50ae3092d10188d7d10403c5e6a1e5205ae Mon Sep 17 00:00:00 2001 From: thelovekesh Date: Thu, 10 Nov 2022 18:55:56 +0530 Subject: [PATCH 5/9] Update regex to replace class names added by block editor --- tests/php/test-class-amp-core-block-handler.php | 9 ++++++--- .../php/validation/test-class-amp-validation-manager.php | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/php/test-class-amp-core-block-handler.php b/tests/php/test-class-amp-core-block-handler.php index 433de8c51bd..63c5591ac5f 100644 --- a/tests/php/test-class-amp-core-block-handler.php +++ b/tests/php/test-class-amp-core-block-handler.php @@ -352,13 +352,16 @@ public function test_ampify_gallery_block( $original_block_content, $expected_bl $actual = preg_replace( '/ data-id="\d+"/', '', $actual ); // Remove `is-layout-flex` class name injected by block editor layout styles. - $actual = preg_replace( '/(?<= class=")is-layout-flex /', '', $actual ); + $actual = preg_replace( '/\s*(?<= class=")?is-layout-flex\s*/', '', $actual ); // Remove `wp-block-gallery-` class by block_core_gallery_render() - $actual = preg_replace( '/(?<= class=")wp-block-gallery-\w+ /', '', $actual ); + $actual = preg_replace( '/\s*(?<= class=")?wp-block-gallery-\w+\s*/', '', $actual ); // Remove class name injected by gutenberg_render_layout_support_flag(). - $actual = preg_replace( '/(?<= class=")wp-container-\w+ /', '', $actual ); + $actual = preg_replace( '/\s*(?<= class=")?wp-container-\w+\s*/', '', $actual ); + + // Remove whitespace from the class attribute in the end. + $actual = preg_replace( '/ class=""/', '', $actual ); $this->assertEqualMarkup( $expected, $actual ); } diff --git a/tests/php/validation/test-class-amp-validation-manager.php b/tests/php/validation/test-class-amp-validation-manager.php index 60987485e3e..aefa17489bd 100644 --- a/tests/php/validation/test-class-amp-validation-manager.php +++ b/tests/php/validation/test-class-amp-validation-manager.php @@ -1394,10 +1394,10 @@ public function test_add_block_source_comments( $content, $expected, $query ) { $rendered_block = do_blocks( AMP_Validation_Manager::add_block_source_comments( $content ) ); // Remove `is-layout-flex` class name injected by block editor layout styles. - $rendered_block = preg_replace( '/(?<= class=")is-layout-flex /', '', $rendered_block ); + $rendered_block = preg_replace( '/\s*(?<= class=")?is-layout-flex\s*/', '', $rendered_block ); // Remove class name injected by gutenberg_render_layout_support_flag(). - $rendered_block = preg_replace( '/(?<= class=")wp-container-\w+ /', '', $rendered_block ); + $rendered_block = preg_replace( '/\s*(?<= class=")?wp-container-\w+\s*/', '', $rendered_block ); $expected = str_replace( [ From 1b24bedb87a33c999c28f937e1e29418fb80df3a Mon Sep 17 00:00:00 2001 From: thelovekesh Date: Thu, 10 Nov 2022 19:11:06 +0530 Subject: [PATCH 6/9] Fix PHPStan error for video id type --- includes/embeds/class-amp-vimeo-embed-handler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/embeds/class-amp-vimeo-embed-handler.php b/includes/embeds/class-amp-vimeo-embed-handler.php index 959054f5b40..d005e960d70 100644 --- a/includes/embeds/class-amp-vimeo-embed-handler.php +++ b/includes/embeds/class-amp-vimeo-embed-handler.php @@ -146,7 +146,7 @@ private function get_video_id_from_url( $url ) { $video_id = $matches[1]; } - return $video_id; + return (int) $video_id; } /** From 100790b68aa25666c604b7a60edb86e7f62bc624 Mon Sep 17 00:00:00 2001 From: thelovekesh Date: Thu, 10 Nov 2022 19:13:49 +0530 Subject: [PATCH 7/9] Update host name in tests --- tests/php/test-class-amp-link-sanitizer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/test-class-amp-link-sanitizer.php b/tests/php/test-class-amp-link-sanitizer.php index a96ea64ce2a..c1c00b6ea78 100644 --- a/tests/php/test-class-amp-link-sanitizer.php +++ b/tests/php/test-class-amp-link-sanitizer.php @@ -339,7 +339,7 @@ public function get_test_is_frontend_url() { false, ], 'different_host' => [ - 'https://example.org/', + 'https://cdn.foo.org/', false, ], 'different_path' => [ From b1325400cddb7244530f106af898116de16c2e7b Mon Sep 17 00:00:00 2001 From: thelovekesh Date: Thu, 10 Nov 2022 20:34:01 +0530 Subject: [PATCH 8/9] Install twentytwenty theme during e2e tests init --- tests/e2e/config/bootstrap.js | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/e2e/config/bootstrap.js b/tests/e2e/config/bootstrap.js index f0662cf138d..be2762224dd 100644 --- a/tests/e2e/config/bootstrap.js +++ b/tests/e2e/config/bootstrap.js @@ -260,6 +260,7 @@ async function setupThemesAndPlugins() { await deactivatePlugin( 'do-not-allow-amp-validate-capability' ); await installTheme( 'hestia' ); + await installTheme( 'twentytwenty' ); // Ensure that twentytwenty theme is installed. await activateTheme( 'twentytwenty' ); } From 2c39c36292e77cc6c4a22e0d6b6914d2c290d6d0 Mon Sep 17 00:00:00 2001 From: thelovekesh Date: Thu, 10 Nov 2022 20:35:49 +0530 Subject: [PATCH 9/9] Add scoll to element to keep it on viewport --- tests/e2e/specs/admin/analytics-options.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/e2e/specs/admin/analytics-options.js b/tests/e2e/specs/admin/analytics-options.js index b32a9711dbb..adc479714c1 100644 --- a/tests/e2e/specs/admin/analytics-options.js +++ b/tests/e2e/specs/admin/analytics-options.js @@ -30,6 +30,8 @@ describe( 'AMP analytics options', () => { await expect( '.amp-analytics-entry' ).countToBe( 2 ); await expect( page ).toFill( '#amp-analytics-entry-2 input', 'googleanalytics-2' ); + await scrollToElement( { selector: '#amp-analytics-add-entry' } ); + // Add third entry. await expect( page ).toClick( '#amp-analytics-add-entry' ); await expect( '.amp-analytics-entry' ).countToBe( 3 );