Merge branches 'html-api/gutenberg-wpcs-issues' and 'html-api/support…

…-button-element' into html-api/add-get-set-inner-outer-contents
WordPress · Aug 3, 2023 · 0c23a14 · 0c23a14
2 parents 4ea240e + b791c27
commit 0c23a14
Show file tree

Hide file tree

Showing 6 changed files with 175 additions and 27 deletions.
diff --git a/src/wp-includes/html-api/class-wp-html-open-elements.php b/src/wp-includes/html-api/class-wp-html-open-elements.php
@@ -115,6 +115,15 @@ public function has_element_in_specific_scope( $tag_name, $termination_list ) {
 			if ( $node->node_name === $tag_name ) {
 				return true;
 			}
+
+			switch ( $node->node_name ) {
+				case 'HTML':
+					return false;
+			}
+
+			if ( in_array( $node->node_name, $termination_list, true ) ) {
+				return true;
+			}
 		}
 
 		return false;
@@ -175,19 +184,7 @@ public function has_element_in_list_item_scope( $tag_name ) { // phpcs:ignore Va
 	 * @return bool Whether given element is in scope.
 	 */
 	public function has_element_in_button_scope( $tag_name ) {
-		return $this->has_element_in_specific_scope(
-			$tag_name,
-			array(
-
-				/*
-				 * Because it's not currently possible to encounter
-				 * one of the termination elements, they don't need
-				 * to be listed here. If they were, they would be
-				 * unreachable and only waste CPU cycles while
-				 * scanning through HTML.
-				 */
-			)
-		);
+		return $this->has_element_in_specific_scope( $tag_name, array( 'BUTTON' ) );
 	}
 
 	/**
@@ -394,6 +391,10 @@ public function after_element_push( $item ) {
 		 * cases where the precalculated value needs to change.
 		 */
 		switch ( $item->node_name ) {
+			case 'BUTTON':
+				$this->has_p_in_button_scope = false;
+				break;
+
 			case 'P':
 				$this->has_p_in_button_scope = true;
 				break;
@@ -419,6 +420,10 @@ public function after_element_pop( $item ) {
 		 * cases where the precalculated value needs to change.
 		 */
 		switch ( $item->node_name ) {
+			case 'BUTTON':
+				$this->has_p_in_button_scope = $this->has_element_in_button_scope( 'P' );
+				break;
+
 			case 'P':
 				$this->has_p_in_button_scope = $this->has_element_in_button_scope( 'P' );
 				break;

diff --git a/src/wp-includes/html-api/class-wp-html-processor-state.php b/src/wp-includes/html-api/class-wp-html-processor-state.php
@@ -107,6 +107,19 @@ class WP_HTML_Processor_State {
 	 */
 	public $context_node = null;
 
+	/**
+	 * The frameset-ok flag indicates if a `FRAMESET` element is allowed in the current state.
+	 *
+	 * > The frameset-ok flag is set to "ok" when the parser is created. It is set to "not ok" after certain tokens are seen.
+	 *
+	 * @since 6.4.0
+	 *
+	 * @see https://html.spec.whatwg.org/#frameset-ok-flag
+	 *
+	 * @var bool
+	 */
+	public $frameset_ok = true;
+
 	/**
 	 * Constructor - creates a new and empty state value.
 	 *

diff --git a/src/wp-includes/html-api/class-wp-html-processor.php b/src/wp-includes/html-api/class-wp-html-processor.php
@@ -349,7 +349,13 @@ public function get_last_error() {
 	 */
 	public function next_tag( $query = null ) {
 		if ( null === $query ) {
-			return $this->step();
+			while ( $this->step() ) {
+				if ( ! $this->is_tag_closer() ) {
+					return true;
+				}
+			}
+
+			return false;
 		}
 
 		if ( is_string( $query ) ) {
@@ -366,7 +372,13 @@ public function next_tag( $query = null ) {
 		}
 
 		if ( ! ( array_key_exists( 'breadcrumbs', $query ) && is_array( $query['breadcrumbs'] ) ) ) {
-			return $this->step();
+			while ( $this->step() ) {
+				if ( ! $this->is_tag_closer() ) {
+					return true;
+				}
+			}
+
+			return false;
 		}
 
 		if ( isset( $query['tag_closers'] ) && 'visit' === $query['tag_closers'] ) {
@@ -383,7 +395,7 @@ public function next_tag( $query = null ) {
 
 		$crumb  = end( $breadcrumbs );
 		$target = strtoupper( $crumb );
-		while ( $this->step() ) {
+		while ( $match_offset > 0 && $this->step() ) {
 			if ( $target !== $this->get_tag() ) {
 				continue;
 			}
@@ -395,7 +407,7 @@ public function next_tag( $query = null ) {
 				}
 
 				$crumb = prev( $breadcrumbs );
-				if ( false === $crumb && 0 === --$match_offset ) {
+				if ( false === $crumb && 0 === --$match_offset && ! $this->is_tag_closer() ) {
 					return true;
 				}
 			}
@@ -510,6 +522,22 @@ private function step_in_body() {
 		$op       = "{$op_sigil}{$tag_name}";
 
 		switch ( $op ) {
+			/*
+			 * > A start tag whose tag name is "button"
+			 */
+			case '+BUTTON':
+				if ( $this->state->stack_of_open_elements->has_element_in_scope( 'BUTTON' ) ) {
+					// @TODO: Indicate a parse error once it's possible. This error does not impact the logic here.
+					$this->generate_implied_end_tags();
+					$this->state->stack_of_open_elements->pop_until( 'BUTTON' );
+				}
+
+				$this->reconstruct_active_formatting_elements();
+				$this->insert_html_element( $this->current_token );
+				$this->state->frameset_ok = false;
+
+				return true;
+
 			/*
 			 * > A start tag whose tag name is one of: "address", "article", "aside",
 			 * > "blockquote", "center", "details", "dialog", "dir", "div", "dl",
@@ -535,15 +563,20 @@ private function step_in_body() {
 			 * > "menu", "nav", "ol", "pre", "search", "section", "summary", "ul"
 			 */
 			case '-BLOCKQUOTE':
+			case '-BUTTON':
 			case '-DIV':
 			case '-FIGCAPTION':
 			case '-FIGURE':
 				if ( ! $this->state->stack_of_open_elements->has_element_in_scope( $tag_name ) ) {
+					// @TODO: Report parse error.
 					// Ignore the token.
 					return $this->step();
 				}
 
 				$this->generate_implied_end_tags();
+				if ( $this->state->stack_of_open_elements->current_node()->node_name !== $tag_name ) {
+					// @TODO: Record parse error: this error doesn't impact parsing.
+				}
 				$this->state->stack_of_open_elements->pop_until( $tag_name );
 				return true;
 
@@ -923,7 +956,7 @@ private function generate_implied_end_tags( $except_for_this_element = null ) {
 		}
 	}
 
-	/*
+	/**
 	 * Closes elements that have implied end tags, thoroughly.
 	 *
 	 * See the HTML specification for an explanation why this is

diff --git a/tests/phpunit/tests/html-api/wpHtmlProcessorBreadcrumbs.php b/tests/phpunit/tests/html-api/wpHtmlProcessorBreadcrumbs.php
@@ -39,6 +39,7 @@ public function data_single_tag_of_supported_elements() {
 			'A',
 			'B',
 			'BIG',
+			'BUTTON',
 			'CODE',
 			'DIV',
 			'EM',
@@ -111,7 +112,6 @@ public function data_unsupported_elements() {
 			'BLINK', // Deprecated
 			'BODY',
 			'BR',
-			'BUTTON',
 			'CANVAS',
 			'CAPTION',
 			'CENTER', // Neutralized

diff --git a/tests/phpunit/tests/html-api/wpHtmlProcessorSemanticRules.php b/tests/phpunit/tests/html-api/wpHtmlProcessorSemanticRules.php
@@ -16,6 +16,110 @@ class Tests_HtmlApi_WpHtmlProcessorSemanticRules extends WP_UnitTestCase {
 	 * RULES FOR "IN BODY" MODE
 	 *******************************************************************/
 
+	/**
+	 * Verifies that when encountering an end tag for which there is no corresponding
+	 * element in scope, that it skips the tag entirely.
+	 *
+	 * @ticket {TICKET_NUMBER}
+	 *
+	 * @since 6.4.0
+	 *
+	 * @throws Exception
+	 */
+	public function test_in_body_skips_unexpected_button_closer() {
+		$p = WP_HTML_Processor::createFragment( '<div>Test</button></div>' );
+
+		$p->step();
+		$this->assertEquals( 'DIV', $p->get_tag(), 'Did not stop at initial DIV tag.' );
+		$this->assertFalse( $p->is_tag_closer(), 'Did not find that initial DIV tag is an opener.' );
+
+		$this->assertTrue( $p->step(), 'Found no further tags when it should have found the closing DIV' );
+		$this->assertEquals( 'DIV', $p->get_tag(), "Did not skip unexpected BUTTON; stopped at {$p->get_tag()}." );
+		$this->assertTrue( $p->is_tag_closer(), 'Did not find that the terminal DIV tag is a closer.' );
+	}
+
+	/**
+	 * Verifies insertion of a BUTTON element when no existing BUTTON is already in scope.
+	 *
+	 * @ticket 58961
+	 *
+	 * @since 6.4.0
+	 *
+	 * @throws WP_HTML_Unsupported_Exception
+	 */
+	public function test_in_body_button_with_no_button_in_scope() {
+		$p = WP_HTML_Processor::createFragment( '<div><p>Click the button <button one>here</button>!</p></div><button two>not here</button>' );
+
+		$this->assertTrue( $p->next_tag( 'BUTTON' ), 'Could not find expected first button.' );
+		$this->assertTrue( $p->get_attribute( 'one' ), 'Failed to match expected attribute on first button.' );
+		$this->assertSame( array( 'HTML', 'BODY', 'DIV', 'P', 'BUTTON' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting for first button.' );
+
+		$this->assertTrue( $p->next_tag( 'BUTTON' ), 'Could not find expected second button.' );
+		$this->assertTrue( $p->get_attribute( 'two' ), 'Failed to match expected attribute on second button.' );
+		$this->assertSame( array( 'HTML', 'BODY', 'BUTTON' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting for second button.' );
+	}
+
+	/**
+	 * Verifies what when inserting a BUTTON element, when a BUTTON is already in scope,
+	 * that the open button is closed with all other elements inside of it.
+	 *
+	 * @ticket 58961
+	 *
+	 * @since 6.4.0
+	 *
+	 * @throws WP_HTML_Unsupported_Exception
+	 */
+	public function test_in_body_button_with_button_in_scope_as_parent() {
+		$p = WP_HTML_Processor::createFragment( '<div><p>Click the button <button one>almost<button two>here</button>!</p></div><button three>not here</button>' );
+
+		$this->assertTrue( $p->next_tag( 'BUTTON' ), 'Could not find expected first button.' );
+		$this->assertTrue( $p->get_attribute( 'one' ), 'Failed to match expected attribute on first button.' );
+		$this->assertSame( array( 'HTML', 'BODY', 'DIV', 'P', 'BUTTON' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting for first button.' );
+
+		$this->assertTrue( $p->next_tag( 'BUTTON' ), 'Could not find expected second button.' );
+		$this->assertTrue( $p->get_attribute( 'two' ), 'Failed to match expected attribute on second button.' );
+		$this->assertSame( array( 'HTML', 'BODY', 'DIV', 'P', 'BUTTON' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting for second button.' );
+
+		$this->assertTrue( $p->next_tag( 'BUTTON' ), 'Could not find expected third button.' );
+		$this->assertTrue( $p->get_attribute( 'three' ), 'Failed to match expected attribute on third button.' );
+		$this->assertSame( array( 'HTML', 'BODY', 'BUTTON' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting for third button.' );
+	}
+
+	/**
+	 * Verifies what when inserting a BUTTON element, when a BUTTON is already in scope,
+	 * that the open button is closed with all other elements inside of it, even if the
+	 * BUTTON in scope is not a direct parent of the new BUTTON element.
+	 *
+	 * @ticket 58961
+	 *
+	 * @since 6.4.0
+	 *
+	 * @throws WP_HTML_Unsupported_Exception
+	 */
+	public function test_in_body_button_with_button_in_scope_as_ancestor() {
+		$p = WP_HTML_Processor::createFragment( '<div><button one><p>Click the button <span><button two>here</button>!</span></p></div><button three>not here</button>' );
+
+		// This button finds itself normally nesting inside the DIV.
+		$this->assertTrue( $p->next_tag( 'BUTTON' ), 'Could not find expected first button.' );
+		$this->assertTrue( $p->get_attribute( 'one' ), 'Failed to match expected attribute on first button.' );
+		$this->assertSame( array( 'HTML', 'BODY', 'DIV', 'BUTTON' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting for first button.' );
+
+		/*
+		 * Because the second button appears while a BUTTON is in scope, it generates implied end tags and closes
+		 * the BUTTON, P, and SPAN elements. It looks like the BUTTON is inside the SPAN, but we have another case
+		 * of an unexpected closing SPAN tag because the SPAN was closed by the second BUTTON. This element finds
+		 * itself a child of the most-recent open element above the most-recent BUTTON, or the DIV.
+		 */
+		$this->assertTrue( $p->next_tag( 'BUTTON' ), 'Could not find expected second button.' );
+		$this->assertTrue( $p->get_attribute( 'two' ), 'Failed to match expected attribute on second button.' );
+		$this->assertSame( array( 'HTML', 'BODY', 'DIV', 'BUTTON' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting for second button.' );
+
+		// The third button is back to normal, because everything has been implicitly or explicitly closed by now.
+		$this->assertTrue( $p->next_tag( 'BUTTON' ), 'Could not find expected third button.' );
+		$this->assertTrue( $p->get_attribute( 'three' ), 'Failed to match expected attribute on third button.' );
+		$this->assertSame( array( 'HTML', 'BODY', 'BUTTON' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting for third button.' );
+	}
+
 	/*
 	 * Verifies that when "in body" and encountering "any other end tag"
 	 * that the HTML processor ignores the end tag if there's a special
@@ -57,7 +161,7 @@ public function test_in_body_any_other_end_tag_with_unclosed_non_special_element
 		$this->assertSame( 'CODE', $p->get_tag(), "Expected to start test on CODE element but found {$p->get_tag()} instead." );
 		$this->assertSame( array( 'HTML', 'BODY', 'DIV', 'SPAN', 'CODE' ), $p->get_breadcrumbs(), 'Failed to produce expected DOM nesting.' );
 
-		$this->assertTrue( $p->next_tag(), 'Failed to advance past CODE tag to expected SPAN closer.' );
+		$this->assertTrue( $p->step(), 'Failed to advance past CODE tag to expected SPAN closer.' );
 		$this->assertTrue( $p->is_tag_closer(), 'Expected to find closing SPAN, but found opener instead.' );
 		$this->assertSame( array( 'HTML', 'BODY', 'DIV' ), $p->get_breadcrumbs(), 'Failed to advance past CODE tag to expected DIV opener.' );
 

diff --git a/tests/phpunit/tests/html-api/wpHtmlSupportRequiredOpenElements.php b/tests/phpunit/tests/html-api/wpHtmlSupportRequiredOpenElements.php
@@ -176,8 +176,6 @@ public function test_has_element_in_button_scope_needs_support() {
 		$this->ensure_support_is_added_everywhere( 'FOREIGNOBJECT' );
 		$this->ensure_support_is_added_everywhere( 'DESC' );
 		$this->ensure_support_is_added_everywhere( 'TITLE' );
-
-		$this->ensure_support_is_added_everywhere( 'BUTTON' );
 	}
 
 	/**
@@ -218,9 +216,6 @@ public function test_after_element_pop_must_maintain_p_in_button_scope_flag() {
 		$this->ensure_support_is_added_everywhere( 'FOREIGNOBJECT' );
 		$this->ensure_support_is_added_everywhere( 'DESC' );
 		$this->ensure_support_is_added_everywhere( 'TITLE' );
-
-		// This element is specific to BUTTON scope.
-		$this->ensure_support_is_added_everywhere( 'BUTTON' );
 	}
 
 	/**
@@ -261,8 +256,6 @@ public function test_after_element_push_must_maintain_p_in_button_scope_flag() {
 		$this->ensure_support_is_added_everywhere( 'FOREIGNOBJECT' );
 		$this->ensure_support_is_added_everywhere( 'DESC' );
 		$this->ensure_support_is_added_everywhere( 'TITLE' );
-
-		$this->ensure_support_is_added_everywhere( 'BUTTON' );
 	}
 
 	/**