Changeset 57649

Timestamp:

02/17/2024 03:26:43 PM (14 months ago)

Author:

swissspidy

Message:

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.

Location:

trunk

Files:

• src/wp-includes/interactivity-api/class-wp-interactivity-api-directives-processor.php (modified) (1 diff)
• src/wp-includes/interactivity-api/class-wp-interactivity-api.php (modified) (1 diff)
• tests/phpunit/tests/interactivity-api/wpInteractivityAPI.php (modified) (2 diffs)
• tests/phpunit/tests/interactivity-api/wpInteractivityAPIDirectivesProcessor.php (modified) (2 diffs)

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

@@ -182 +182 @@
 
     /**
+     * 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.
      *

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

@@ -236 +236 @@
             $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;
-                break;
+                $p->skip_to_tag_closer();
+                continue;
             }
 

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

@@ -12 +12 @@
  * @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.
@@ -509 +509 @@
 
     /**
-     * Tests that the `process_directives` returns the same HTML if it finds an
-     * SVG tag.
-     *
-     * @ticket 60356
-     *
-     * @covers ::process_directives
-     */
-    public function test_process_directives_doesnt_change_html_if_contains_svgs() {
-        $this->interactivity->state( 'myPlugin', array( 'id' => 'some-id' ) );
+     * Tests that the `process_directives` process the HTML outside a SVG tag.
+     *
+     * @ticket 60517
+     *
+     * @covers ::process_directives
+     */
+    public function test_process_directives_changes_html_if_contains_svgs() {
+        $this->interactivity->state(
+            'myPlugin',
+            array(
+                'id'    => 'some-id',
+                'width' => '100',
+            )
+        );
         $html           = '
-            <div data-wp-bind--id="myPlugin::state.id">
-                <svg height="100" width="100">
+            <header>
+                <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>
+                </svg>
+                <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
-     *
-     * @covers ::process_directives
-     */
-    public function test_process_directives_doesnt_change_html_if_contains_math() {
-        $this->interactivity->state( 'myPlugin', array( 'id' => 'some-id' ) );
+     * @ticket 60517
+     *
+     * @covers ::process_directives
+     */
+    public function test_process_directives_change_html_if_contains_math() {
+        $this->interactivity->state(
+            'myPlugin',
+            array(
+                'id'   => 'some-id',
+                'math' => 'ml-id',
+            )
+        );
         $html           = '
-            <div data-wp-bind--id="myPlugin::state.id">
-                <math>
+            <header>
+                <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' ) );
     }

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

@@ -12 +12 @@
  * @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
@@ -779 +779 @@
         $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() );
+    }
 }