Interactivity API: Skip instead of bail out if HTML contains SVG or MATH.

Addresses an issue with server-side processing of directives when there is e.g. an SVG icon a navigation menu.

Props cbravobernal, westonruter, dmsnell, swissspidy.
Fixes #60517.

git-svn-id: https://develop.svn.wordpress.org/trunk@57649 602fd350-edb4-49c9-b593-d223f7449a82

src/wp-includes/interactivity-api/class-wp-interactivity-api-directives-processor.php

@@ -180,6 +180,39 @@ final class WP_Interactivity_API_Directives_Processor extends WP_HTML_Tag_Proces
 		return array( $opener_tag, $closer_tag );
 	}
 
+	/**
+	 * Skips processing the content between tags.
+	 *
+	 * It positions the cursor in the closer tag of the foreign element, if it
+	 * exists.
+	 *
+	 * This function is intended to skip processing SVG and MathML inner content
+	 * instead of bailing out the whole processing.
+	 *
+	 * @since 6.5.0
+	 *
+	 * @access private
+	 *
+	 * @return bool Whether the foreign content was successfully skipped.
+	 */
+	public function skip_to_tag_closer(): bool {
+		$depth    = 1;
+		$tag_name = $this->get_tag();
+		while ( $depth > 0 && $this->next_tag(
+			array(
+				'tag_name'    => $tag_name,
+				'tag_closers' => 'visit',
+			)
+		) ) {
+			if ( $this->has_self_closing_flag() ) {
+				continue;
+			}
+			$depth += $this->is_tag_closer() ? -1 : 1;
+		}
+
+		return 0 === $depth;
+	}
+
 	/**
 	 * Finds the matching closing tag for an opening tag.
 	 *

src/wp-includes/interactivity-api/class-wp-interactivity-api.php

@@ -235,9 +235,14 @@ final class WP_Interactivity_API {
 		while ( $p->next_tag( array( 'tag_closers' => 'visit' ) ) ) {
 			$tag_name = $p->get_tag();
 
+			/*
+			 * Directives inside SVG and MATH tags are not processed,
+			 * as they are not compatible with the Tag Processor yet.
+			 * We still process the rest of the HTML.
+			 */
 			if ( 'SVG' === $tag_name || 'MATH' === $tag_name ) {
-				$unbalanced = true;
+				$p->skip_to_tag_closer();
-				break;
+				continue;
 			}
 
 			if ( $p->is_tag_closer() ) {

tests/phpunit/tests/interactivity-api/wpInteractivityAPI.php

@@ -11,7 +11,7 @@
  *
  * @coversDefaultClass WP_Interactivity_API
  */
-class Tests_WP_Interactivity_API extends WP_UnitTestCase {
+class Tests_Interactivity_API_WpInteractivityAPI extends WP_UnitTestCase {
 	/**
 	 * Instance of WP_Interactivity_API.
 	 *
@@ -508,50 +508,131 @@ class Tests_WP_Interactivity_API extends WP_UnitTestCase {
 	}
 
 	/**
-	 * Tests that the `process_directives` returns the same HTML if it finds an
+	 * Tests that the `process_directives` process the HTML outside a SVG tag.
-	 * SVG tag.
 	 *
-	 * @ticket 60356
+	 * @ticket 60517
 	 *
 	 * @covers ::process_directives
 	 */
-	public function test_process_directives_doesnt_change_html_if_contains_svgs() {
+	public function test_process_directives_changes_html_if_contains_svgs() {
-		$this->interactivity->state( 'myPlugin', array( 'id' => 'some-id' ) );
+		$this->interactivity->state(
+			'myPlugin',
+			array(
+				'id'    => 'some-id',
+				'width' => '100',
+			)
+		);
 		$html           = '
-			<div data-wp-bind--id="myPlugin::state.id">
+			<header>
-				<svg height="100" width="100">
+				<svg height="100" data-wp-bind--width="myPlugin::state.width">
+					<title>Red Circle</title>
 					<circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red" />
 				</svg>
-			</div>
+				<div data-wp-bind--id="myPlugin::state.id"></div>
+				<div data-wp-bind--id="myPlugin::state.width"></div>
+			</header>
 		';
 		$processed_html = $this->interactivity->process_directives( $html );
 		$p              = new WP_HTML_Tag_Processor( $processed_html );
-		$p->next_tag();
+		$p->next_tag( 'svg' );
+		$this->assertNull( $p->get_attribute( 'width' ) );
+		$p->next_tag( 'div' );
+		$this->assertEquals( 'some-id', $p->get_attribute( 'id' ) );
+		$p->next_tag( 'div' );
+		$this->assertEquals( '100', $p->get_attribute( 'id' ) );
+	}
+
+	/**
+	 * Tests that the `process_directives` does not process the HTML
+	 * inside SVG tags.
+	 *
+	 * @ticket 60517
+	 *
+	 * @covers ::process_directives
+	 */
+	public function test_process_directives_does_not_change_inner_html_in_svgs() {
+		$this->interactivity->state(
+			'myPlugin',
+			array(
+				'id' => 'some-id',
+			)
+		);
+		$html           = '
+			<header>
+				<svg height="100">
+					<circle cx="50" cy="50" r="40" stroke="black" stroke-width="3" fill="red" />
+					<g data-wp-bind--id="myPlugin::state.id" />
+				</svg>
+			</header>
+		';
+		$processed_html = $this->interactivity->process_directives( $html );
+		$p              = new WP_HTML_Tag_Processor( $processed_html );
+		$p->next_tag( 'div' );
 		$this->assertNull( $p->get_attribute( 'id' ) );
 	}
 
 	/**
-	 * Tests that the `process_directives` returns the same HTML if it finds an
+	 * Tests that the `process_directives` process the HTML outside the
 	 * MathML tag.
 	 *
-	 * @ticket 60356
+	 * @ticket 60517
 	 *
 	 * @covers ::process_directives
 	 */
-	public function test_process_directives_doesnt_change_html_if_contains_math() {
+	public function test_process_directives_change_html_if_contains_math() {
-		$this->interactivity->state( 'myPlugin', array( 'id' => 'some-id' ) );
+		$this->interactivity->state(
+			'myPlugin',
+			array(
+				'id'   => 'some-id',
+				'math' => 'ml-id',
+			)
+		);
 		$html           = '
-			<div data-wp-bind--id="myPlugin::state.id">
+			<header>
-				<math>
+				<math data-wp-bind--id="myPlugin::state.math">
 					<mi>x</mi>
 					<mo>=</mo>
 					<mi>1</mi>
 				</math>
-			</div>
+				<div data-wp-bind--id="myPlugin::state.id"></div>
+			</header>
 		';
 		$processed_html = $this->interactivity->process_directives( $html );
 		$p              = new WP_HTML_Tag_Processor( $processed_html );
-		$p->next_tag();
+		$p->next_tag( 'math' );
+		$this->assertNull( $p->get_attribute( 'id' ) );
+		$p->next_tag( 'div' );
+		$this->assertEquals( 'some-id', $p->get_attribute( 'id' ) );
+	}
+
+	/**
+	 * Tests that the `process_directives` does not process the HTML
+	 * inside MathML tags.
+	 *
+	 * @ticket 60517
+	 *
+	 * @covers ::process_directives
+	 */
+	public function test_process_directives_does_not_change_inner_html_in_math() {
+		$this->interactivity->state(
+			'myPlugin',
+			array(
+				'id' => 'some-id',
+			)
+		);
+		$html           = '
+			<header>
+				<math data-wp-bind--id="myPlugin::state.math">
+					<mrow data-wp-bind--id="myPlugin::state.id" />
+					<mi>x</mi>
+					<mo>=</mo>
+					<mi>1</mi>
+				</math>
+			</header>
+		';
+		$processed_html = $this->interactivity->process_directives( $html );
+		$p              = new WP_HTML_Tag_Processor( $processed_html );
+		$p->next_tag( 'div' );
 		$this->assertNull( $p->get_attribute( 'id' ) );
 	}
 

tests/phpunit/tests/interactivity-api/wpInteractivityAPIDirectivesProcessor.php

@@ -11,7 +11,7 @@
  *
  * @coversDefaultClass WP_Interactivity_API_Directives_Processor
  */
-class Tests_WP_Interactivity_API_Directives_Processor extends WP_UnitTestCase {
+class Tests_Interactivity_API_WpInteractivityAPIDirectivesProcessor extends WP_UnitTestCase {
 	/**
 	 * Tests the `get_content_between_balanced_template_tags` method on template
 	 * tags.
@@ -778,4 +778,51 @@ class Tests_WP_Interactivity_API_Directives_Processor extends WP_UnitTestCase {
 		$p->next_tag( array( 'tag_closers' => 'visit' ) );
 		$this->assertFalse( $p->next_balanced_tag_closer_tag() );
 	}
+
+	/**
+	 * Tests that skip_to_tag_closer skips to the next tag,
+	 * independant of the content.
+	 *
+	 * @ticket 60517
+	 *
+	 * @covers ::skip_to_tag_closer
+	 */
+	public function test_skip_to_tag_closer() {
+		$content = '<div><span>Not closed</div>';
+		$p       = new WP_Interactivity_API_Directives_Processor( $content );
+		$p->next_tag();
+		$this->assertTrue( $p->skip_to_tag_closer() );
+		$this->assertTrue( $p->is_tag_closer() );
+		$this->assertEquals( 'DIV', $p->get_tag() );
+	}
+
+	/**
+	 * Tests that skip_to_tag_closer does not skip to the
+	 * next tag if there is no closing tag.
+	 *
+	 * @ticket 60517
+	 *
+	 * @covers ::skip_to_tag_closer
+	 */
+	public function test_skip_to_tag_closer_bails_not_closed() {
+		$content = '<div>Not closed parent';
+		$p       = new WP_Interactivity_API_Directives_Processor( $content );
+		$p->next_tag();
+		$this->assertFalse( $p->skip_to_tag_closer() );
+	}
+
+	/**
+	 * Tests that skip_to_tag_closer does not skip to the next
+	 * tag if the closing tag is different from the current tag.
+	 *
+	 * @ticket 60517
+	 *
+	 * @covers ::skip_to_tag_closer
+	 */
+	public function test_skip_to_tag_closer_bails_different_tags() {
+		$content = '<div></span>';
+		$p       = new WP_Interactivity_API_Directives_Processor( $content );
+		$p->next_tag();
+		$this->assertFalse( $p->skip_to_tag_closer() );
+	}
 }