Repository navigation
Conversation
…g flag. A `/` inside a tag is only a self-closing flag when immediately followed by `>`. When an attribute is directly preceded by a `/`, e.g. `<g /attr>`, removing the attribute leaves `<g />`, introducing a self-closing flag that wasn't in the input and changing the tag's semantics. These tests currently fail and document the expected behavior.
…flag. Inside a tag, a `/` is ignored unless it's immediately followed by `>`, in which case it's the self-closing flag. When an attribute is directly preceded by a `/`, e.g. `<g /attr>`, removing just the attribute's own span left `<g />`, introducing a self-closing flag that wasn't in the input and changing the semantics of the tag. Removals now also consume any `/` characters immediately preceding the attribute. This is safe because a `/` can only appear before an attribute as a separator: tag names and attribute names end at `/`, and inside an unquoted value it's part of the value rather than preceding an attribute. The same logic applies when removing duplicated attributes.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
…ntax. Broaden the self-closing flag test to assert that the tag name, the remaining attributes, and the following content are all unchanged, and add cases where removing an attribute along with the `/` preceding it would otherwise merge the tag name or a neighboring attribute with what follows, e.g. `<g/a="x"b>` becoming `<gb>`. Also add a test for combining set_attribute() and remove_attribute() on the same tag when the removal extends back to the end of the tag name, which is where new attributes are inserted. These tests currently fail.
…n attribute. Consuming the `/` characters before a removed attribute introduced two problems of its own: 1. Only a quoted attribute value can be directly followed by another attribute, e.g. `<g/a="x"b>`. Removing `/a="x"` outright merged the tag name with the following attribute: `<gb>`. Likewise `<g c/a="x"b>` became `<g cb>`. When the consumed slashes were the only separator and an attribute follows, a single space now takes their place. 2. When the removed attribute directly follows the tag name, e.g. `<g/a>`, the removal now starts at the same offset where new attributes are inserted by set_attribute(). Lexical updates were sorted by start and then by text, which ordered the removal (empty text) before the insertion; applying the removal first advanced the copy cursor past the insertion point and produced corrupted output such as `<g> b="x"/a>`. Zero-length insertions now sort before consuming replacements at the same offset.
…e-attribute-self-closing-flag # Conflicts: # src/wp-includes/html-api/class-wp-html-tag-processor.php
sirreal
added a commit
to sirreal/wordpress-develop
that referenced
this pull request
Oct 19, 2026
Adds tests that remove one attribute from tags where slashes separate attributes, follow the tag name, end an unquoted value, or form the self-closing flag, and where another attribute directly follows a quoted value. Each test re-parses the updated HTML and asserts that the tag name, the self-closing flag, the remaining attributes, and the following text are unchanged, so a stray or missing separator fails the test even when the expected string is wrong. Ported from WordPress#13208, with the expected strings updated to the single space this change writes in place of a slash-preceded attribute. See #65967.
sirreal
added a commit
to sirreal/wordpress-develop
that referenced
this pull request
Oct 19, 2026
…ame. `set_attribute()` and `add_class()` insert new attributes directly after the tag name. When only a slash separates the tag name from an attribute that is then removed, the insertion and the removal are a single byte apart. These tests apply both in either order, with a self-closing flag, with another attribute following, and with two removals of different attributes, then re-parse the updated HTML and assert the tag name, self-closing flag, attributes, and following text. Ported from WordPress#13208, with the expected strings updated to this change's output and a re-parse check added. See #65967.
sirreal
added a commit
to sirreal/wordpress-develop
that referenced
this pull request
Oct 19, 2026
…tribute with a space. Documents that a `/` directly before an attribute can only be a separator, why leaving it before `>` would add a self-closing flag, and which readers treat the single-space replacement as a removal. The explanation of why the `/` is always a separator comes from WordPress#13208. See #65967.
Member
Author
|
Closing in favor of #12518, which now includes the tests from this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Inside a tag, a
/is ignored unless it's immediately followed by>, in which case it's the self-closing flag. When an attribute is directly preceded by a/, removing just the attribute's own span can leave that/adjacent to the>, introducing a self-closing flag that wasn't in the input:Other affected shapes:
<g/attr>→<g/>,<g a/b>(removeb) →<g a/>,<g a="x"/b>(removeb) →<g a="x"/>, and duplicated attributes like<g /a /a>→<g / />.Fix
Attribute removals now also consume any
/characters immediately preceding the attribute. This is safe because a/can only appear directly before an attribute as a separator: tag names and attribute names terminate at/, and inside an unquoted value/is part of the value rather than preceding an attribute.Two consequences of consuming the slashes are handled:
<g/a="x"b>. Removing/a="x"outright would merge the tag name with the following attribute (<gb>);<g c/a="x"b>would become<g cb>. When the consumed slashes were the only separator and an attribute immediately follows, a single space takes their place:<g b>,<g c b>.<g/a>), the removal now starts at the same offset whereset_attribute()inserts new attributes. Lexical updates were ordered by start then by text, which placed the removal (empty text) before the insertion and corrupted the output (<g> b="x"/a>). Zero-length insertions now sort before consuming replacements at the same offset.The same logic applies when removing duplicated attributes.
Whitespace between a
/and the attribute is intentionally left alone (<g / attr>→<g / >), since/ >is not a self-closing flag.Testing
test_remove_attribute_preserves_tag_semantics: data provider of slash/quote/equals shapes; asserts exact output, then re-parses and checks the tag name, self-closing flag, remaining attribute set, and following text are unchanged.test_set_and_remove_attribute_at_tag_name_end: combinations ofset_attribute()/add_class()andremove_attribute()where the removal reaches the tag-name end.Beyond the unit tests, a combinatorial fuzzer over malformed tag shapes (separators
' ',/,//,/,''…; attribute shapesa,a=b,a="b",a=/,a=b/,=,=a,a ="b"…; closers>,/>,/ >,//>…; up to three attributes; duplicates) appliedremove_attribute,set_attribute,add_class,remove_classand combinations, then re-parsed the output and diffed the token stream against the expected change. Before this PR: ~83k of 1.6M checks diverged. After: 0 of 2.3M, excluding one unrelated pre-existing class noted below.Out of scope
The fuzzer also surfaced a pre-existing
set_attribute( $name, true )issue: a bare boolean name emitted directly before whitespace followed by=merges with it. E.g.<x =>+set_attribute( 'b', true )→<x b =>, which parses asb=""and drops the=attribute. That's independent of removal and not addressed here.Trac ticket:
https://core.trac.wordpress.org/ticket/65967
🤖 Generated with Claude Code