Changeset 57649

Timestamp:

02/17/2024 03:26:43 PM (3 years 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() );
+        }
 }