Prevent schema.org duplicates

Check if schema.org script is present. Add schema.org only if it doesn't exist.
ampproject · Mar 9, 2018 · c66bfc4 · c66bfc4
1 parent 1357330
commit c66bfc4
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 57 deletions.
diff --git a/includes/amp-post-template-actions.php b/includes/amp-post-template-actions.php
@@ -14,6 +14,7 @@ function amp_post_template_init_hooks() {
 	add_action( 'amp_post_template_head', 'amp_post_template_add_scripts' );
 	add_action( 'amp_post_template_head', 'amp_post_template_add_fonts' );
 	add_action( 'amp_post_template_head', 'amp_post_template_add_boilerplate_css' );
+	add_action( 'amp_post_template_head', 'amp_print_schemaorg_metadata' );
 	add_action( 'amp_post_template_head', 'amp_add_generator_metadata' );
 	add_action( 'amp_post_template_css', 'amp_post_template_add_styles', 99 );
 	add_action( 'amp_post_template_data', 'amp_post_template_add_analytics_script' );

diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -794,7 +794,7 @@ public static function get_amp_scripts( $amp_scripts ) {
 	 *
 	 * @param DOMDocument $dom Doc.
 	 */
-	protected static function ensure_required_markup( DOMDocument $dom ) {
+	public static function ensure_required_markup( DOMDocument $dom ) {
 		$head = $dom->getElementsByTagName( 'head' )->item( 0 );
 		if ( ! $head ) {
 			$head = $dom->createElement( 'head' );
@@ -828,14 +828,21 @@ protected static function ensure_required_markup( DOMDocument $dom ) {
 			) );
 			$head->insertBefore( $meta_viewport, $meta_charset->nextSibling );
 		}
-		if ( ! self::schema_org_present( $dom ) ) {
+		// Prevent schema.org duplicates.
+		$schema_org_script = null;
+		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;
+				break;
+			}
+		}
+		if ( ! $schema_org_script ) {
 			$script = $dom->createElement( 'script', wp_json_encode( amp_get_schemaorg_metadata() ) );
 			AMP_DOM_Utils::add_attributes_to_node( $script, array(
 				'type' => 'application/ld+json',
 			) );
 			$head->appendChild( $script );
 		}
-
 		// Ensure rel=canonical link.
 		$rel_canonical = null;
 		foreach ( $head->getElementsByTagName( 'link' ) as $link ) {
@@ -1013,20 +1020,4 @@ public static function prepare_response( $response, $args = array() ) {
 
 		return $response;
 	}
-
-	/**
-	 * Checks if there is already a schema.org script in the head.
-	 *
-	 * @param DOMDocument $dom Representation of the document.
-	 * @return bool
-	 */
-	public static function schema_org_present( DOMDocument $dom ) {
-		$head = $dom->getElementsByTagName( 'head' )->item( 0 );
-		foreach ( $head->getElementsByTagName( 'script' ) as $script ) {
-			if ( 1 === preg_match( '/{"@context":"https?:[\\\]\/[\\\]\/schema\.org/', $script->nodeValue ) ) {
-				return true;
-			}
-		}
-		return false;
-	}
 }
diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php
@@ -332,47 +332,23 @@ public function test_handle_xhr_request() {
 	}
 
 	/**
-	 * Test schema_org_present().
+	 * Test ensure_required_markup().
 	 *
-	 * @dataProvider get_script_data
-	 * @covers AMP_Theme_Support::schema_org_present()
-	 * @param string  $script The value of the script.
-	 * @param boolean $expected The expected result.
+	 * @covers AMP_Theme_Support::ensure_required_markup()
 	 */
-	public function test_schema_org_present( $script, $expected ) {
-		$page = '<html><head><script>%s</script></head><body>Test</body></html>';
+	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>';
 		$dom  = new DOMDocument();
-		$dom->loadHTML( sprintf( $page, $script ) );
-		$this->assertEquals( $expected, AMP_Theme_Support::schema_org_present( $dom ) );
-	}
+		$dom->loadHTML( $page );
+		AMP_Theme_Support::ensure_required_markup( $dom );
+		$this->assertContains( $schema_test_value, $dom->saveHTML() );
 
-	/**
-	 * Data provider for test_schema_org_present().
-	 *
-	 * @return array
-	 */
-	public function get_script_data() {
-		return array(
-			'string_schema_value'       => array(
-				'schema.org',
-				false,
-			),
-			'string_not_schema'         => array(
-				'somethinghere.org',
-				false,
-			),
-			'json_schema_present_https' => array(
-				wp_json_encode( array( '@context' => 'https://schema.org' ) ),
-				true,
-			),
-			'json_schema_present_http'  => array(
-				wp_json_encode( array( '@context' => 'http://schema.org' ) ),
-				true,
-			),
-			'json_schema_not_present'   => array(
-				wp_json_encode( array( '@anothercontext' => 'https://schema.org' ) ),
-				false,
-			),
-		);
+		$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 );
 	}
 }