Prevent Schema.org duplicates logic refactoring

ampproject · Mar 10, 2018 · 8b96d77 · 8b96d77
1 parent c66bfc4
commit 8b96d77
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 19 deletions.
diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -828,19 +828,18 @@ public static function ensure_required_markup( DOMDocument $dom ) {
 			) );
 			$head->insertBefore( $meta_viewport, $meta_charset->nextSibling );
 		}
+
 		// Prevent schema.org duplicates.
-		$schema_org_script = null;
+		$has_schema_org_metadata = false;
 		foreach ( $head->getElementsByTagName( 'script' ) as $script ) {
-			if ( 'application/ld+json' === $script->getAttribute( 'type' ) && preg_match( '/{"@context":"https?:[\\\]\/[\\\]\/schema\.org/', $script->nodeValue ) ) {
-				$schema_org_script = $script->nodeValue;
+			if ( 'application/ld+json' === $script->getAttribute( 'type' ) && false !== strpos( $script->nodeValue, 'schema.org' ) ) {
+				$has_schema_org_metadata = true;
 				break;
 			}
 		}
-		if ( ! $schema_org_script ) {
+		if ( ! $has_schema_org_metadata ) {
 			$script = $dom->createElement( 'script', wp_json_encode( amp_get_schemaorg_metadata() ) );
-			AMP_DOM_Utils::add_attributes_to_node( $script, array(
-				'type' => 'application/ld+json',
-			) );
+			$script->setAttribute( 'type', 'application/ld+json' );
 			$head->appendChild( $script );
 		}
 		// Ensure rel=canonical link.

diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php
@@ -334,21 +334,44 @@ public function test_handle_xhr_request() {
 	/**
 	 * Test ensure_required_markup().
 	 *
+	 * @dataProvider get_script_data
 	 * @covers AMP_Theme_Support::ensure_required_markup()
 	 */
-	public function test_ensure_required_markup() {
-		$schema_test_value = '<script type="application/ld+json">{"@context":"http:\/\/schema.org"';
-
-		$page = '<html><head></head><body>Test</body></html>';
+	public function test_ensure_required_markup( $script, $expected ) {
+		$page = '<html><head><script type="application/ld+json">%s</script></head><body>Test</body></html>';
 		$dom  = new DOMDocument();
-		$dom->loadHTML( $page );
+		$dom->loadHTML( sprintf( $page, $script ) );
 		AMP_Theme_Support::ensure_required_markup( $dom );
-		$this->assertContains( $schema_test_value, $dom->saveHTML() );
-
-		$page = '<html><head<script type="application/ld+json">{"@context":"http:\/\/schema.org","publisher":{"@type":"Organization","name":"Test Blog"}}</script>></head><body>Test</body></html>';
-		$dom  = new DOMDocument();
-		$dom->loadHTML( $page );
-		AMP_Theme_Support::ensure_required_markup( $dom );
-		$this->assertEquals( substr_count( $dom->saveHTML(), $schema_test_value ), 1 );
+		$this->assertEquals( $expected, substr_count( $dom->saveHTML(), 'schema.org' ) );
+	}
+	/**
+	 * Data provider for test_ensure_required_markup().
+	 *
+	 * @return array
+	 */
+	public function get_script_data() {
+		return array(
+			'nothing_present'                => array(
+				'',
+				1,
+			),
+			'schema_present'                 => array(
+				wp_json_encode( array( '@context' => 'https://schema.org' ) ),
+				1,
+			),
+			'schema_present_another_context' => array(
+				wp_json_encode( array( '@anothercontext' => 'https://schema.org' ) ),
+				1,
+			),
+			'schema_present_not_escaped'     => array(
+				'{"@context":"http://schema.org"',
+				1,
+			),
+			'json_schema_present_http'       => array(
+				wp_json_encode( array( '@context' => 'http://schema.org' ) ),
+				1,
+			),
+		);
 	}
 }
+