<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[56941] trunk: HTML API: Avoid calling subclass method while internally scanning in Tag Processor.</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { white-space: pre-line; overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta" style="font-size: 105%">
<dt style="float: left; width: 6em; font-weight: bold">Revision</dt> <dd><a style="font-weight: bold" href="https://core.trac.wordpress.org/changeset/56941">56941</a><script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","description":"Review this Commit","action":{"@type":"ViewAction","url":"https://core.trac.wordpress.org/changeset/56941","name":"Review Commit"}}</script></dd>
<dt style="float: left; width: 6em; font-weight: bold">Author</dt> <dd>SergeyBiryukov</dd>
<dt style="float: left; width: 6em; font-weight: bold">Date</dt> <dd>2023-10-16 14:00:01 +0000 (Mon, 16 Oct 2023)</dd>
</dl>
<pre style='padding-left: 1em; margin: 2em 0; border-left: 2px solid #ccc; line-height: 1.25; font-size: 105%; font-family: sans-serif'>HTML API: Avoid calling subclass method while internally scanning in Tag Processor.
After modifying tags in the HTML API, the Tag Processor backs up to before the tag being modified and then re-parses its attributes. This saves on the code complexity involved in applying updates, which have already been transformed to {U+201C}lexical updates{U+201D} by the time they are applied.
In order to do that, `::get_updated_html()` called `::next_tag()` to reuse its logic. However, as a public method, subclasses may change the behavior of that method, and the HTML Processor does just this. It maintains an HTML stack of open elements and when the Tag Processor calls this method to re-scan a tag and its attributes, it leads to a broken stack.
This commit replaces the call to `::next_tag()` with a more appropriate reapplication of its internal parsing logic to rescan the tag name and its attributes. Given the limited nature of what's occurring in `::get_updated_html()`, this should bring with it certain guarantees that no HTML structure is being changed (that structure will only be changed by subclasses like the HTML Processor).
Follow-up to <a href="https://core.trac.wordpress.org/changeset/56274">[56274]</a>, <a href="https://core.trac.wordpress.org/changeset/56702">[56702]</a>.
Props dmsnell, zieladam, nicolefurlan.
Fixes <a href="https://core.trac.wordpress.org/ticket/59607">#59607</a>.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunksrcwpincludeshtmlapiclasswphtmltagprocessorphp">trunk/src/wp-includes/html-api/class-wp-html-tag-processor.php</a></li>
<li><a href="#trunktestsphpunittestshtmlapiwpHtmlProcessorBreadcrumbsphp">trunk/tests/phpunit/tests/html-api/wpHtmlProcessorBreadcrumbs.php</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunksrcwpincludeshtmlapiclasswphtmltagprocessorphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/src/wp-includes/html-api/class-wp-html-tag-processor.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/src/wp-includes/html-api/class-wp-html-tag-processor.php 2023-10-16 04:41:13 UTC (rev 56940)
+++ trunk/src/wp-includes/html-api/class-wp-html-tag-processor.php 2023-10-16 14:00:01 UTC (rev 56941)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -2270,6 +2270,7 @@
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span><span class="cx" style="display: block; padding: 0 10px"> * @since 6.2.0
</span><span class="cx" style="display: block; padding: 0 10px"> * @since 6.2.1 Shifts the internal cursor corresponding to the applied updates.
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * @since 6.4.0 No longer calls subclass method `next_tag()` after updating HTML.
</ins><span class="cx" style="display: block; padding: 0 10px"> *
</span><span class="cx" style="display: block; padding: 0 10px"> * @return string The processed HTML.
</span><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -2303,25 +2304,26 @@
</span><span class="cx" style="display: block; padding: 0 10px"> * Rewind before the tag name starts so that it's as if the cursor didn't
</span><span class="cx" style="display: block; padding: 0 10px"> * move; a call to `next_tag()` will reparse the recently-updated attributes
</span><span class="cx" style="display: block; padding: 0 10px"> * and additional calls to modify the attributes will apply at this same
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * location.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * location, but in order to avoid issues with subclasses that might add
+ * behaviors to `next_tag()`, the internal methods should be called here
+ * instead.
</ins><span class="cx" style="display: block; padding: 0 10px"> *
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * It's important to note that in this specific place there will be no change
+ * because the processor was already at a tag when this was called and it's
+ * rewinding only to the beginning of this very tag before reprocessing it
+ * and its attributes.
+ *
</ins><span class="cx" style="display: block; padding: 0 10px"> * <p>Previous HTML<em>More HTML</em></p>
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * ^ | back up by the length of the tag name plus the opening <
- * \<-/ back up by strlen("em") + 1 ==> 3
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * ↑ │ back up by the length of the tag name plus the opening <
+ * └←─┘ back up by strlen("em") + 1 ==> 3
</ins><span class="cx" style="display: block; padding: 0 10px"> */
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">-
- // Store existing state so it can be restored after reparsing.
- $previous_parsed_byte_count = $this->bytes_already_parsed;
- $previous_query = $this->last_query;
-
- // Reparse attributes.
</del><span class="cx" style="display: block; padding: 0 10px"> $this->bytes_already_parsed = $before_current_tag;
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- $this->next_tag();
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ $this->parse_next_tag();
+ // Reparse the attributes.
+ while ( $this->parse_next_attribute() ) {
+ continue;
+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // Restore previous state.
- $this->bytes_already_parsed = $previous_parsed_byte_count;
- $this->parse_query( $previous_query );
-
</del><span class="cx" style="display: block; padding: 0 10px"> return $this->html;
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span></span></pre></div>
<a id="trunktestsphpunittestshtmlapiwpHtmlProcessorBreadcrumbsphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/tests/phpunit/tests/html-api/wpHtmlProcessorBreadcrumbs.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/tests/phpunit/tests/html-api/wpHtmlProcessorBreadcrumbs.php 2023-10-16 04:41:13 UTC (rev 56940)
+++ trunk/tests/phpunit/tests/html-api/wpHtmlProcessorBreadcrumbs.php 2023-10-16 14:00:01 UTC (rev 56941)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -411,6 +411,53 @@
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /**
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * Ensures that updating tag's attributes doesn't shift the current position
+ * in the input HTML document.
+ *
+ * @since 6.4.0
+ *
+ * @ticket 59607
+ *
+ * @covers WP_HTML_Tag_Processor::get_updated_html
+ */
+ public function test_remains_stable_when_editing_attributes() {
+ $p = WP_HTML_Processor::create_fragment( '<div><button>First<button><b here>Second' );
+ $p->next_tag( array( 'breadcrumbs' => array( 'BUTTON', 'B' ) ) );
+
+ $this->assertSame(
+ array( 'HTML', 'BODY', 'DIV', 'BUTTON', 'B' ),
+ $p->get_breadcrumbs(),
+ 'Found the wrong nested structure at the matched tag.'
+ );
+
+ $p->set_attribute( 'a-name', 'a-value' );
+
+ $this->assertTrue(
+ $p->get_attribute( 'here' ),
+ 'Should have found the B tag but could not find expected "here" attribute.'
+ );
+
+ $this->assertSame(
+ array( 'HTML', 'BODY', 'DIV', 'BUTTON', 'B' ),
+ $p->get_breadcrumbs(),
+ 'Found the wrong nested structure at the matched tag.'
+ );
+
+ $p->get_updated_html();
+
+ $this->assertTrue(
+ $p->get_attribute( 'here' ),
+ 'Should have stayed at the B tag but could not find expected "here" attribute.'
+ );
+
+ $this->assertSame(
+ array( 'HTML', 'BODY', 'DIV', 'BUTTON', 'B' ),
+ $p->get_breadcrumbs(),
+ 'Found the wrong nested structure at the matched tag after updating attributes.'
+ );
+ }
+
+ /**
</ins><span class="cx" style="display: block; padding: 0 10px"> * Ensures that the ability to set attributes isn't broken by the HTML Processor.
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span><span class="cx" style="display: block; padding: 0 10px"> * @since 6.4.0
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -417,7 +464,7 @@
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span><span class="cx" style="display: block; padding: 0 10px"> * @ticket 58517
</span><span class="cx" style="display: block; padding: 0 10px"> *
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * @covers WP_HTML_Processor::set_attribute
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * @covers WP_HTML_Tag_Processor::set_attribute
</ins><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="cx" style="display: block; padding: 0 10px"> public function test_can_modify_attributes_after_finding_tag() {
</span><span class="cx" style="display: block; padding: 0 10px"> $p = WP_HTML_Processor::create_fragment( '<div><figure><img><figcaption>test</figcaption></figure>' );
</span></span></pre>
</div>
</div>
</body>
</html>