HTML API: get_attribute_names_with_prefix() should agree with enqueued attribute updates.

[dmsnell]

The get_attribute_names_with_prefix() method only looks at the parsed attributes for a tag, meaning that it will return stale data once new attributes have been set or existing ones have been removed. It should not need to wait for ->get_updated_html() to report the proper values, and should always agree with ->get_attribute()

<?php
$p = new WP_HTML_Tag_Processor( '<div>' );
$p->next_tag();
$p->set_attribute( 'id', 'test' );
var_dump( $p->get_attribute_names_with_prefix( '' ) );
// array(0) {}

$p->get_updated_html();
var_dump( $p->get_attribute_names_with_prefix( '' ) );
// array(1) { [0] => string(2) "id" }

Fixed: get_attribute_names_with_prefix() now agrees with enqueued attribute updates
The method now returns real-time attribute names that reflect set_attribute() and remove_attribute() calls without needing to call get_updated_html() first.

Trac ticket: https://core.trac.wordpress.org/ticket/64567

---

[whaze]

Testing:

<?php
$p = new WP_HTML_Tag_Processor( '<div>' );
$p->next_tag();
$p->set_attribute( 'id', 'test' );
// Now correctly returns ['id']
var_dump( $p->get_attribute_names_with_prefix( '' ) );

Rest

On Fri, Jan 30, 2026, 03:22 Jerome B. *@*.*> wrote:

Fixed: get_attribute_names_with_prefix() now agrees with enqueued
attribute updates
The method now returns real-time attribute names that reflect
set_attribute() and remove_attribute() calls without needing to call
get_updated_html() first.

Trac ticket: https://core.trac.wordpress.org/ticket/64567

---

---

You can view, comment on, or merge this pull request online at:

​https://github.com/WordPress/wordpress-develop/pull/10828

Commit Summary

• 6ee4b36 <​https://github.com/WordPress/wordpress-develop/pull/10828/commits/6ee4b368a55001bec4515f73a785564419a79e47> Fixed: get_attribute_names_with_prefix() now agrees with enqueued attribute updates

File Changes

(1 file <​https://github.com/WordPress/wordpress-develop/pull/10828/files>)

• *M* src/wp-includes/html-api/class-wp-html-tag-processor.php <​https://github.com/WordPress/wordpress-develop/pull/10828/files#diff-0c2d7bb49853cc0faafd5412a25ee90e8cbb4f9293e7c3bb308494aba7d08272> (35)

Patch Links:

• ​https://github.com/WordPress/wordpress-develop/pull/10828.patch
• ​https://github.com/WordPress/wordpress-develop/pull/10828.diff

—
Reply to this email directly, view it on GitHub
<​https://github.com/WordPress/wordpress-develop/pull/10828>, or
unsubscribe
<​https://github.com/notifications/unsubscribe-auth/BXZWJXFX6GBMB55QFJVVC6L4JMIFJAVCNFSM6AAAAACTNCJCJWVHI2DSMVQWIX3LMV43ASLTON2WKOZTHA3TINJZGA2DMOI>
.
You are receiving this because you are subscribed to this thread.Message
ID: *@*.*>

[phpbits]

Tested using this PR: ​https://github.com/WordPress/wordpress-develop/pull/10828

add_filter( 'render_block_core/quote', 'test_attribute_names_with_prefix', 10, 2 );

function test_attribute_names_with_prefix( string $block_content, array $block ): string {

	$p = new WP_HTML_Tag_Processor( '<div>' );
	$p->next_tag();
	$p->set_attribute( 'id', 'test' );
	echo '<pre>';
	print_r( $p->get_attribute_names_with_prefix( '' ) );
	echo '</pre>';

	$processor = new WP_HTML_Tag_Processor( $block_content );
	$processor->next_tag();
	$processor->set_attribute( 'id', 'test' );
	echo '<pre>';
	print_r( $processor->get_attribute_names_with_prefix( '' ) );
	echo '</pre>';

	die();

	return $processor->get_updated_html();
}

The code above returns the correct attributes here:

Array
(
    [0] => id
)
Array
(
    [0] => class
    [1] => id
)

[dmsnell]

Thanks everyone for your help. We seem to have a growing amount of uncoordinated activity, with two proposed patches and one manual test.

What would be great is if we combined our efforts and added new unit tests alongside the existing Tag Processor tests for get_attribute_names_with_prefix(). If we have those, then we won’t need to perform manual testing — we’ll only need to review the test code itself.

Getting the test cases in, including but potentially not limited to adding an attribute, removing an attribute, and changing an attribute, with or without existing duplicates, might reveal some differences between the different proposals.

[audrasjb]

As per today's 7.0 pre-RC1 bug scrub:

We have two patches, but they need to be merged into one, and @dmsnell dmsnell proposed some additional tests. We're good to punt it to 7.1.

Following the 7.0 pre-RC1 scrub note ("two patches… need to be merged into one, and @dmsnell proposed some additional tests"), here are unit tests for the path that actually exercises this ticket — querying attribute names without first flushing updates through get_updated_html(). The existing test_get_attribute_names_with_prefix_returns_attribute_added_by_set_attribute() calls get_updated_html() first, which masks the bug.

These pass on this branch and fail on trunk (proving they catch the reported behavior):

/**
         * Ensures that an attribute added via set_attribute() is reported by
         * get_attribute_names_with_prefix() without first flushing the updates
         * through get_updated_html().
         *
         * @ticket 64567
         *
         * @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix
         */
        public function test_get_attribute_names_with_prefix_reflects_set_attribute_without_flushing() {
                $processor = new WP_HTML_Tag_Processor( '<div>Test</div>' );
                $processor->next_tag();
                $processor->set_attribute( 'id', 'test' );

                $this->assertSame(
                        array( 'id' ),
                        $processor->get_attribute_names_with_prefix( '' ),
                        'Did not report an attribute enqueued via set_attribute() before the updates were flushed.'
                );
        }

        /**
         * Ensures that an attribute removed via remove_attribute() is no longer
         * reported by get_attribute_names_with_prefix() without first flushing the
         * updates through get_updated_html().
         *
         * @ticket 64567
         *
         * @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix
         */
        public function test_get_attribute_names_with_prefix_reflects_removed_attribute_without_flushing() {
                $processor = new WP_HTML_Tag_Processor( '<div id="main" data-foo="bar">Test</div>' );
                $processor->next_tag();
                $processor->remove_attribute( 'data-foo' );

                $this->assertSame(
                        array(),
                        $processor->get_attribute_names_with_prefix( 'data-' ),
                        'Reported an attribute that had been enqueued for removal before the updates were flushed.'
                );
                $this->assertSame(
                        array( 'id' ),
                        $processor->get_attribute_names_with_prefix( '' ),
                        'Did not reflect a removed attribute in the full attribute name list before flushing.'
                );
        }

        /**
         * Ensures that a class enqueued via add_class() surfaces the `class`
         * attribute in get_attribute_names_with_prefix() without first flushing the
         * updates through get_updated_html().
         *
         * @ticket 64567
         *
         * @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix
         */
        public function test_get_attribute_names_with_prefix_reflects_added_class_without_flushing() {
                $processor = new WP_HTML_Tag_Processor( '<div>Test</div>' );
                $processor->next_tag();
                $processor->add_class( 'highlight' );

                $this->assertSame(
                        array( 'class' ),
                        $processor->get_attribute_names_with_prefix( 'class' ),
                        'Did not report the class attribute enqueued via add_class() before the updates were flushed.'
                );
        }

        /**
         * Ensures get_attribute_names_with_prefix() agrees with get_attribute()
         * after pending updates, returning each name once with no stale entries.
         *
         * @ticket 64567
         *
         * @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix
         */
        public function test_get_attribute_names_with_prefix_agrees_with_get_attribute_after_updates() {
                $processor = new WP_HTML_Tag_Processor( '<div data-keep="1" data-drop="2">Test</div>' );
                $processor->next_tag();
                $processor->set_attribute( 'data-keep', 'updated' );
                $processor->set_attribute( 'data-add', 'new' );
                $processor->remove_attribute( 'data-drop' );

                $names = $processor->get_attribute_names_with_prefix( 'data-' );
                sort( $names );

                $this->assertSame(
                        array( 'data-add', 'data-keep' ),
                        $names,
                        'Enqueued additions, updates, and removals were not reflected accurately, or a name was duplicated.'
                );

                foreach ( $names as $name ) {
                        $this->assertNotNull(
                                $processor->get_attribute( $name ),
                                "get_attribute_names_with_prefix() reported \"{$name}\" but get_attribute() did not agree."
                        );
                }
        }

They go in tests/phpunit/tests/html-api/wpHtmlTagProcessor.php. Happy to push as a commit if that's easier.

[motylanogha]

Unit tests for get_attribute_names_with_prefix() (no-flush path). Pass against PR #10828, fail on trunk.

[motylanogha]

Added unit tests for the no-flush path of WP_HTML_Tag_Processor::get_attribute_names_with_prefix() — querying attribute names after set_attribute(), remove_attribute(), and add_class() without first calling get_updated_html() (the existing test masks the issue by flushing first). Four tests covering set/remove/add-class and agreement with get_attribute(). They pass against the patch in PR #10828 and fail on trunk. Patch attached (64567-tests.diff); same tests posted on the PR.

[sachinrajcp123]

Uploaded a patch for this ticket.

[anupkankale]

Thanks for tackling this, @whaze — the approach of merging the enqueued lexical_updates on top of the parsed attributes is exactly right, and it lines this method up with how get_attribute() already behaves.

One edge case I hit while testing: the second loop skips integer keys with is_int( $name ), but lexical_updates can also hold the string key 'modifiable text' (enqueued by set_modifiable_text()). Since that key is a string, it isn't skipped, so for any matching prefix it gets returned as if it were an attribute name.

Repro:

$processor = new WP_HTML_Tag_Processor( '<script id="x">old</script>' );
$processor->next_tag();
$processor->set_modifiable_text( 'new content' );

$processor->get_attribute_names_with_prefix( );
actual: array( 'id', 'modifiable text' )
expected: array( 'i

A one-line guard in the skip condition handles it:
foreach ( $this->lexical_updates as $name => $update ) {

Skip positional updates and the modifiable-text update; neither is an attribute.

if ( is_int( $name )

'modifiable text' === $name ) { continue;

}
...

}

If it helps, I've written unit tests for the no-flush path — set / remov/ change attribute, a a cross-check that thereported names agree with get_attribute(), plus a regression test for the 'modifiable text' case above. They fail on trunk and pass with your fix (the modifiable-text odded). Happy to pushthem to this branch or open a PR against it — whichever you'd prefer.

<details>
<summary>5 unit tests for the no-flush path (click to expand)</summary>

/

• @ticket 64567
• @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix */

public function test_get_attribute_names_with_prefix_reflects_set_attribute_without_flushing() {

$processor = new WP_HTML_Tag_Processor( '<div data-foo="bar">Test</d);
$processor->next_tag();
$processor->set_attribute( 'data-test-id', '14' );

$this->assertSame(

array( 'data-foo', 'data-test-id' ),
$processor->get_attribute_names_with_prefix( 'data-' ),
'Enqueued attribute added via set_attribute() was not reflecflushing updates.'

);

}

/

• @ticket 64567
• @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix */

public function test_get_attribute_names_with_prefix_reflects_remove_attribute_without_flushing() {

$processor = new WP_HTML_Tag_Processor( '<div data-foo="bar" data-keep="1">Test</div>' );
$processor->next_tag();
$processor->remove_attribute( 'data-foo' );

$this->assertSame(

array( 'data-keep' ),
$processor->get_attribute_names_with_prefix( 'data-' ),
'Enqueued attribute removal via remove_attribute() was not rbefore flushing updates.'

);

}

/

• @ticket 64567
• @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix */

public function test_get_attribute_names_with_prefix_reflects_added_class_without_flushing() {

$processor = new WP_HTML_Tag_Processor( '<div data-foo="bar">Test</d);
$processor->next_tag();
$processor->add_class( 'highlight' );

$this->assertSame(

array( 'class' ),
$processor->get_attribute_names_with_prefix( 'class' ),
'Enqueued class attribute created via add_class() was not re flushing updates.'

);

}

/

• @ticket 64567
• @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix */

public function test_get_attribute_names_with_prefix_agrees_with_get_attribute_after_updates() {

$processor = new WP_HTML_Tag_Processor( '<div data-a="1" data-b="2" data-c="3">Test</div>' );
$processor->next_tag();
$processor->set_attribute( 'data-b', 'updated' ); Change an existattribute.
$processor->remove_attribute( 'data-c' ); Remove an existattribute.
$processor->set_attribute( 'data-d', 'new' ); Add a new attri

$names = $processor->get_attribute_names_with_prefix( 'data-' );
sort( $names );

$this->assertSame(

array( 'data-a', 'data-b', 'data-d' ),
$names,
'Reported attribute names did not reflect the enqueued changflushing updates.'

);

foreach ( $names as $name ) {

$this->assertNotNull(

$processor->get_attribute( $name ),
"get_attribute_names_with_prefix() reported '{$name} reports it as absent."

);

}

$this->assertNull(

$processor->get_attribute( 'data-c' ),
'Removed attribute data-c should be null via get_attribute()

);
$this->assertNotContains(

'data-c',
$names,
'Removed attribute data-c should not appear in get_attribute_names_with_prefix().'

);

}

/

• @ticket 64567
• @covers WP_HTML_Tag_Processor::get_attribute_names_with_prefix */

public function test_get_attribute_names_with_prefix_ignores_modifiable_text_update() {

$processor = new WP_HTML_Tag_Processor( '<script id="x">old</script>
$processor->next_tag();
$processor->set_modifiable_text( 'new content' );

$this->assertSame(

array( 'id' ),
$processor->get_attribute_names_with_prefix( ),
"The internal 'modifiable text' update key leaked as an attr

);

}

</details>

[motylanogha]

Good catch, @anupkankale — confirmed. The modifiable text entry lives in the same lexical_updates array as attribute changes but under a string key, so the is_int() skip in PR #10828 lets it leak. I reproduced it against the PR:

$processor = new WP_HTML_Tag_Processor( '<script id="x">old</script>' );
$processor->next_tag();
$processor->set_modifiable_text( 'new content' );
$processor->get_attribute_names_with_prefix( '' );
// PR #10828:  array( 'id', 'modifiable text' )   <-- leak
// expected:   array( 'id' )

Your one-line guard is the right fix and matches the existing style:

foreach ( $this->lexical_updates as $name => $update ) {
        // Skip positional updates and the modifiable-text update; neither is an attribute.
        if ( is_int( $name ) || 'modifiable text' === $name ) {
                continue;
        }
        // ...
}

Rather than open a competing test patch, I've folded a regression test for exactly this case into the existing no-flush test set from comment:10 / comment:11, so there's a single consolidated diff to review (refreshed 64567-tests.diff). It now covers set / remove / change attribute, add_class, agreement with get_attribute(), and the modifiable text leak.

Verified matrix:

• trunk: the four no-flush tests fail (the reported bug), the modifiable-text test passes (trunk never merges updates).
• PR #10828 as-is: the four pass, the modifiable-text test fails (reproduces the leak).
• PR #10828 + the guard above: all five pass.

@whaze — would you like to add the guard line to the PR, or should I push it along with the test onto the branch?

[motylanogha]

Refreshed: adds a 5th test covering the 'modifiable text' leak (comment:12). No-flush path for get_attribute_names_with_prefix() — set / remove / change attribute, add_class, agreement with get_attribute(), and modifiable-text regression. Pass against PR #10828 + the one-line guard; fail on trunk.

[luisherranz]

While prototyping an HTML Sanitizer on top of the HTML API (​draft PR, Trac ticket), Fable ran into a downstream symptom of this ticket in WP_HTML_Processor::serialize_token() that's worth folding into the fix and its tests.

serialize_token() iterates get_attribute_names_with_prefix( '' ) (the stale list this ticket describes) but reads each value through get_attribute(), which does reflect enqueued updates. The two methods disagreeing produces mangled serialization exactly when the name set changes. All four cases on trunk:

<?php
$p = WP_HTML_Processor::create_fragment( '<div onclick="alert(1)" class="x">Text</div>' );
$p->next_tag();
$p->remove_attribute( 'onclick' );
echo $p->serialize_token();
// Actual:   <div onclick class="x">   <-- removal degrades to a value-less attribute
// Expected: <div class="x">

$p = WP_HTML_Processor::create_fragment( '<div class="x">Text</div>' );
$p->next_tag();
$p->set_attribute( 'id', 'new' );
echo $p->serialize_token();
// Actual:   <div class="x">           <-- addition is silently dropped
// Expected: <div class="x" id="new">

// Changing an existing value composes correctly, because the *name* survives:
// set_attribute( 'class', 'changed' )  => <div class="changed">  OK
// add_class( 'extra' )                 => <div class="x extra">  OK

The removal case is the concerning one: remove_attribute( 'onclick' ) followed by serialize_token() emits <div onclick class="x">. The removed name survives via the stale list, get_attribute() returns null for it, and the emitter treats non-string values as boolean attributes. Filtering-then-serializing is exactly the composition that content filters built on the HTML API will use, so this arguably raises the severity: the API can re-emit an attribute a caller explicitly removed.

The good news is that ​PR #10828 plus the 'modifiable text' guard from comment:12 should resolve both broken cases with no changes to serialize_token() itself: once the name list agrees with the enqueued updates, removals are omitted and additions are picked up (their values already come from get_attribute()).

Since serialize_token() is where the disagreement becomes visible in output, a regression test on that path alongside the no-flush tests in 64567-tests.2.diff would pin the behavior (this one fails on trunk):

<?php
/**
 * Ensures that serialize_token() reflects enqueued attribute updates instead
 * of emitting removed attributes as value-less attributes or omitting added ones.
 *
 * @ticket 64567
 *
 * @covers WP_HTML_Processor::serialize_token
 */
public function test_serialize_token_reflects_enqueued_attribute_updates() {
        $processor = WP_HTML_Processor::create_fragment( '<div onclick="alert(1)" class="x">Text</div>' );
        $processor->next_tag();
        $processor->remove_attribute( 'onclick' );

        $this->assertSame(
                '<div class="x">',
                $processor->serialize_token(),
                'An attribute enqueued for removal was serialized as a value-less attribute.'
        );

        $processor = WP_HTML_Processor::create_fragment( '<div class="x">Text</div>' );
        $processor->next_tag();
        $processor->set_attribute( 'id', 'new' );

        $this->assertStringContainsString(
                'id="new"',
                $processor->serialize_token(),
                'An attribute enqueued via set_attribute() was not serialized.'
        );
}

[motylanogha]

Confirmed on trunk — reproduced all four cases from comment:14 via a standalone harness. serialize_token() iterates get_attribute_names_with_prefix( '' ) but reads each value through get_attribute(), and the two disagree exactly as described:

remove_attribute( 'onclick' )      => <div onclick class="x">   (removed attr re-emitted as boolean)
set_attribute( 'id', 'new' )       => <div class="x">           (addition dropped)
set_attribute( 'class', 'changed') => <div class="changed">     OK
add_class( 'extra' )               => <div class="x extra">     OK

And PR #10828 resolves both broken cases with no change to serialize_token() itself — once the name list agrees with the enqueued updates, the removal is omitted and the addition is picked up:

remove_attribute( 'onclick' )      => <div class="x">
set_attribute( 'id', 'new' )       => <div class="x" id="new">

The proposed regression test fails on trunk and passes with PR #10828, so I've folded it in. I placed it in wpHtmlProcessor-serialize.php (Tests_HtmlApi_WpHtmlProcessor_Serialize) rather than the tag-processor file where the no-flush tests live, since serialize_token() is a WP_HTML_Processor method:

tests/phpunit/tests/html-api/wpHtmlProcessor-serialize.php

@@ -389 +389 @@
                         ),
                 );
         }
+
+        /**
+         * Ensures that serialize_token() reflects enqueued attribute updates instead
+         * of emitting removed attributes as value-less attributes or omitting added ones.
+         *
+         * @ticket 64567
+         *
+         * @covers WP_HTML_Processor::serialize_token
+         */
+        public function test_serialize_token_reflects_enqueued_attribute_updates() {
+                $processor = WP_HTML_Processor::create_fragment( '<div onclick="alert(1)" class="x">Text</div>' );
+                $processor->next_tag();
+                $processor->remove_attribute( 'onclick' );
+
+                $this->assertSame(
+                        '<div class="x">',
+                        $processor->serialize_token(),
+                        'An attribute enqueued for removal was serialized as a value-less attribute.'
+                );
+
+                $processor = WP_HTML_Processor::create_fragment( '<div class="x">Text</div>' );
+                $processor->next_tag();
+                $processor->set_attribute( 'id', 'new' );
+
+                $this->assertStringContainsString(
+                        'id="new"',
+                        $processor->serialize_token(),
+                        'An attribute enqueued via set_attribute() was not serialized.'
+                );
+        }
 }

One note: the 'modifiable text' leak from comment:12 doesn't surface through these particular cases — a <div> doesn't enqueue a modifiable-text update, so this test passes with or without the comment:12 guard. It pins the serialize_token() behavior, but the guard from comment:12/13 still needs to land in PR #10828 separately; it isn't covered here.

[khokansardar]

Following the pre-RC1 scrub note (the two patches needing to be merged into one, plus @dmsnell's requested tests), I've opened a revalidated PR that folds everything together:

​PR #12484

Building on @whaze's work in ​PR #10828, it:

• Keeps the original list structure and drops the associative-array / "efficiently track" comment, per @dmsnell's review.
• Reports newly-added attributes first, matching the order get_updated_html() writes them, and omits attributes enqueued for removal.
• Flushes pending add_class() / remove_class() changes so the class attribute is reflected, mirroring get_attribute().
• Skips the reserved modifiable text update key so a pending content update is never mistaken for an attribute name (the earlier patch only skipped integer keys).
• Adds six unit tests after test_get_attribute_names_with_prefix_returns_attribute_added_by_set_attribute() covering additions, removals, add_class(), get_updated_html() ordering, rejected set_attribute() calls, and agreement with get_attribute().

get_attribute_names_with_prefix() only looked at the parsed attributes for a tag,
so it returned stale data once attributes were set or removed via set_attribute(),
remove_attribute(), add_class(), or remove_class(), without first calling
get_updated_html(). This brings it in line with get_attribute(), which already
reconciles enqueued lexical updates before returning.

Trac ticket: https://core.trac.wordpress.org/ticket/64567

## Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Sonnet 5
Used for: Implementation, debugging (including finding and fixing an attribute-
ordering edge case for multiple newly-added attributes), and test coverage.
All changes were reviewed and directed by me throughout.

[irozum]

The patch attached previously (64567-fix.patch.patch) doesn't apply to current trunk — it references $this->updated_attributes, which was renamed to $this->lexical_updates (and changed its internal representation) since that patch was written.

Opened a PR with a refreshed fix: reconciles get_attribute_names_with_prefix() against pending set_attribute()/remove_attribute()/add_class()/remove_class() updates the same way get_attribute() already does, so the two methods agree as described in the ticket. Also handles ordering when multiple new attributes are added in the same call sequence, and includes test coverage.

Happy to adjust based on feedback.

​https://github.com/WordPress/wordpress-develop/pull/12619

[motylanogha]

Quick triage to help the scrub, since there are now three open PRs, plus verification results for each.

I ran every case discussed on this ticket against current trunk and each PR: the five no-flush cases from 64567-tests.2.diff and the two serialize_token() cases from comment:14 / comment:15.

check	trunk	PR #10828	PR #12484	PR #12619
set_attribute() reflected without flush	FAIL	PASS	PASS	PASS
remove_attribute() excluded without flush	FAIL	PASS	PASS	PASS
add_class() reflected without flush	FAIL	PASS	PASS	PASS
agreement with get_attribute() after mixed updates	FAIL	PASS	PASS	PASS
modifiable text key not leaked as attribute name	PASS	FAIL	PASS	PASS
serialize_token() omits removed attribute	FAIL	PASS	PASS	PASS
serialize_token() emits added attribute	FAIL	PASS	PASS	PASS

Summary:

• ​PR #10828 (@whaze): fixes the core disagreement but still leaks the modifiable text key from comment:12; get_attribute_names_with_prefix( '' ) returns ['id', 'modifiable text'] after set_modifiable_text().
• ​PR #12484 (@khokansardar) and ​PR #12619 (@irozum): functionally equivalent. Both include the guard and pass all seven cases, including the two serialize_token() symptoms from comment:14, which are fixed indirectly once the name list agrees with the enqueued updates.

One gap common to all three: none includes the serialize_token() regression test from comment:15, which pins the user-visible symptom on the WP_HTML_Processor path (an attribute enqueued for removal re-emitted as a value-less attribute). Whichever PR is picked, that test should ride along. Happy to push it onto the chosen branch.

[adrianduffell]

It looks like this is close. @dmsnell, do you have the bandwidth to shepherd this before the 7.1 RC release or would it better to punt it?

[motylanogha]

Replying to adrianduffell: on the "is it close" part, the verification matrix in comment:19 still holds. I re-checked this morning and none of the three PRs has moved since (last commits: #10828 on 30 Jan, #12484 on 11 Jul, #12619 on 21 Jul).

Where that leaves it:

• PR #12484 (@khokansardar) and PR #12619 (@irozum) both pass all seven cases discussed on this ticket, including the 'modifiable text' guard from comment:12 and the two serialize_token() symptoms from comment:14. They are functionally equivalent, so picking one is an editorial call, not a technical one.
• PR #10828 (@whaze) still leaks the 'modifiable text' key and would need the one-line guard before it could be committed.

So there is no outstanding code or test work blocking a commit, only the choice of which PR to take. The single thing not present in any of the three is the serialize_token() regression test from comment:15; it is test-only and can ride along with the commit or land immediately after, so it should not gate the decision either.

If nobody has the bandwidth to shepherd it before RC, punting costs nothing here, the patches are not going stale. But if @dmsnell or another committer can spare a pass, the review surface is small.

[logicrays]

Hello Team,

I reproduced this on trunk (wordpress-develop) using WP_HTML_Tag_Processor, without calling get_updated_html() first.

[1] After set_attribute( 'id', 'test' ) on <div>Test</div>:

• get_attribute( 'id' ) -> 'test'
• get_attribute_names_with_prefix( ) -> array()

[2] After remove_attribute( 'data-foo' ) on <div data-foo="bar">:

• get_attribute( 'data-foo' ) -> null
• get_attribute_names_with_prefix( 'data-' ) -> array( 'data-foo' )

So get_attribute_names_with_prefix() does not yet reflect enqueued lexical updates, while get_attribute() does.

For reference, test_get_attribute_names_with_prefix_returns_attribute_added_by_set_attribute() (@56299) passes on trunk because it calls get_updated_html() before querying attribute names, which does not exercise the behavior reported in this ticket.

Please check the screenshot image.

[motylanogha]

Thanks for the repro, @logicrays, it matches. Your point about test_get_attribute_names_with_prefix_returns_attribute_added_by_set_attribute() calling get_updated_html() before it queries the names, and so never exercising the reported path, is the same gap noted in comment:10 / comment:11, so we are looking at the same thing.

Flagging where the ticket actually stands, so effort does not go into re-confirming the report:

• The behaviour is already fixed in all three open PRs. comment:19 has a case-by-case verification of each against current trunk.
• PR #12484 and PR #12619 pass every case discussed here. PR #10828 still leaks the 'modifiable text' key (comment:12).
• Tests for the no-flush path are attached as 64567-tests.2.diff, and both viable PRs already carry equivalents. The only test not present in any PR is the serialize_token() regression from comment:15.

So the open item is comment:20: whether a committer picks one of the two viable PRs before RC1 on 5 August. If you want to help this land, a review of #12484 versus #12619 would be worth more than another repro, since they are functionally equivalent and the choice between them is the thing currently blocking.

Trac ticket: Core-64567.

Previously, get_attribute_names_with_prefix() was overlooking enqueued attribute and class name updates which occurred before get_updated_html() had been called. This resulted in reporting stale data which might overlook attributes which were added, and might report attributes which were removed.

In this patch, the method now examines the enqueued updates to determine if any of them introduce or remove attributes. It also examines enqueued class updates to ensure that if the class attribute would be added or removed because of them that it will also be properly reported.

[dmsnell]

We seem to have a growing amount of uncoordinated activity

unfortunately this only seems to have grown, which makes it all the more difficult to work on. this ticket was never meant to be blocked technically, other than ensuring that we don’t introduce a mistake.

what’s held me up, beside the normal stack of unrelated things to focus on, is trying to gather code from three PRs, code in comments in the PRs, patches here, and a plethora of status updates re-confirming the problem 🙃

for those who have contributed newly to this: it would be extremely helpful for everyone else if when you commit or leave a comment, that you engage with the body of work already proposed by others. even a little note in a PR description would help.

Alternative to #12484: In this one we do X because of Y for better Z.

That helps me as a maintainer to understand the relationship between the work and the tradeoffs chosen. As it stands, and this is my own inability to keep more abreast of all the ongoing work, I am practically going to have to create a fourth PR loosely inspired by this work. I could have done that originally and pushed it, but did want this to be a learning opportunity for someone. I hope it has been.

---

With that, I have proposed ​PR#12757 as an aggregation of all of the mixed proposals. Thanks to everyone for helping with this, in reporting, in proposing patches, in testing, and in keeping this issue alive.

Once the tests pass I’ll try and get some extra signoff and get this merged for 7.1 still.

Verified this branch against every case raised on #64567, including the ones from the earlier PRs. All seven pass, where trunk passes only one:

check	trunk	this PR
set_attribute() reflected without flush	FAIL	PASS
remove_attribute() excluded without flush	FAIL	PASS
add_class() reflected without flush	FAIL	PASS
agreement with get_attribute() after mixed updates	FAIL	PASS
'modifiable text' key not leaked as an attribute name	PASS	PASS
serialize_token() omits removed attribute	FAIL	PASS
serialize_token() emits added attribute	FAIL	PASS

Method: a standalone harness that loads the HTML API files directly against a checkout, so no test DB is involved. It reproduces trunk's failures exactly as reported in comment:19, then passes 7/7 here.

Two things worth noting.

The 'modifiable text' guard matters and you have it. That case (comment:12, found by @anupkankale) is the one thing PR #10828 still gets wrong: set_modifiable_text() stores a *string* key in lexical_updates, so an is_int()-only skip leaks it as a fake attribute name. Your || 'modifiable text' === $update_name handles it.

The serialize_token() regression test isn't in any PR, including this one. WP_HTML_Processor::serialize_token() iterates get_attribute_names_with_prefix( '' ) but reads values via get_attribute(), so before this fix remove_attribute( 'onclick' ) followed by serialize_token() re-emitted the attribute as a boolean: <div onclick class="x">. @luisherranz reported it in comment:14 while prototyping an HTML Sanitizer, and it's the case that makes this a filtering-safety issue rather than a reporting inconsistency. Your fix resolves it indirectly, but nothing pins it, so a future refactor of the name list could silently reintroduce it on the WP_HTML_Processor path.

It belongs in tests/phpunit/tests/html-api/wpHtmlProcessor-serialize.php rather than the tag processor file, since serialize_token() is a WP_HTML_Processor method. Fails on trunk, passes here:

/**
         * Ensures that serialize_token() reflects enqueued attribute updates instead
         * of emitting removed attributes as value-less attributes or omitting added ones.
         *
         * serialize_token() iterates the names from get_attribute_names_with_prefix( '' )
         * but reads each value through get_attribute(). Before #64567 the name list did not
         * reflect enqueued updates while the values did, so a removed attribute survived in
         * the output as a boolean attribute and an added attribute was dropped entirely.
         *
         * @ticket 64567
         *
         * @covers WP_HTML_Processor::serialize_token
         */
        public function test_serialize_token_reflects_enqueued_attribute_updates() {
                $processor = WP_HTML_Processor::create_fragment( '<div onclick="alert(1)" class="x">Text</div>' );
                $processor->next_tag();
                $processor->remove_attribute( 'onclick' );

                $this->assertSame(
                        '<div class="x">',
                        $processor->serialize_token(),
                        'An attribute enqueued for removal was serialized as a value-less attribute.'
                );

                $processor = WP_HTML_Processor::create_fragment( '<div class="x">Text</div>' );
                $processor->next_tag();
                $processor->set_attribute( 'id', 'new' );

                $this->assertStringContainsString(
                        'id="new"',
                        $processor->serialize_token(),
                        'An attribute enqueued via set_attribute() was not serialized.'
                );
        }

Take it straight into this branch if it's useful — no attribution needed. Also attached to the ticket as 64567-serialize-tests.diff and posted in comment:15.

To be precise about what I checked: the seven behaviours above were verified through the standalone harness, not by running the PHPUnit suite, so the test above is verified as behaviour rather than as a green CI run.

Rebased and squashed for merge prep.

thanks @wppoland

Take it straight into this branch if it's useful — no attribution needed.

I’m going to leave that out for now and let it be part of improving the serialization tests. it’s valuable, but in part because it’s the beta part of the release I want to focus on the core fix, which is get_attribute_names_with_prefix()

thanks @westonruter — I’m going to merge this so we can get it in for the release!

[dmsnell]

In 62960:

HTML API: Respect enqueued updates in get_attribute_names_with_prefix().

Trac ticket: Core-64567.

Previously, get_attribute_names_with_prefix() was overlooking enqueued attribute and class name updates which occurred before get_updated_html() had been called. This resulted in reporting stale data which might overlook attributes which were added, and might report attributes which were removed.

In this patch, the method now examines the enqueued updates to determine if any of them introduce or remove attributes. It also examines enqueued class updates to ensure that if the class attribute would be added or removed because of them that it will also be properly reported.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/12757
Discussed in: https://core.trac.wordpress.org/ticket/64567

Follow-up to [55203].

Props anupkankale, irozum, itzmekhokan, luisherranz, motylanogha, phpbits, sachinrajcp123, whaze.
See #64567.

Merged in [62960]
​https://github.com/wordpress/wordpress-develop/commit/ca644847def89cb8159831332377bc9b13fc9f41

Closed by the merge of ​https://github.com/WordPress/wordpress-develop/pull/12757

Closed by the merge of ​https://github.com/WordPress/wordpress-develop/pull/12757

Closed by the merge of ​https://github.com/WordPress/wordpress-develop/pull/12757

[khokansardar]

Replying to dmsnell:

In 62960:

HTML API: Respect enqueued updates in get_attribute_names_with_prefix().

Trac ticket: Core-64567.

Previously, get_attribute_names_with_prefix() was overlooking enqueued attribute and class name updates which occurred before get_updated_html() had been called. This resulted in reporting stale data which might overlook attributes which were added, and might report attributes which were removed.

In this patch, the method now examines the enqueued updates to determine if any of them introduce or remove attributes. It also examines enqueued class updates to ensure that if the class attribute would be added or removed because of them that it will also be properly reported.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/12757
Discussed in: https://core.trac.wordpress.org/ticket/64567

Follow-up to [55203].

Props anupkankale, irozum, itzmekhokan, luisherranz, motylanogha, phpbits, sachinrajcp123, whaze.
See #64567.

@dmsnell don't know why, but my props name should be @khokansardar

Test-only follow-up to [62960].

[62960] made get_attribute_names_with_prefix() respect enqueued updates, and it landed with 208 lines of tests. All of them are in wpHtmlTagProcessor.php.

The bug had a second surface. WP_HTML_Processor::serialize_token() iterates the names from get_attribute_names_with_prefix( '' ) but reads each value through get_attribute(), so it consumed exactly the stale list the fix corrected. Before [62960] that meant:

• remove_attribute( 'onclick' ) then serialize_token() re-emitted the attribute as a value-less one: <div onclick class="x">
• set_attribute( 'id', 'new' ) then serialize_token() dropped the addition entirely

Both are fixed by [62960], with no change to serialize_token() itself. Neither is covered.

This adds one test to wpHtmlProcessor-serialize.php pinning both directions, so the serialization path cannot silently regress if the name-list logic is touched again.

### Verification

Run against tests/phpunit/tests/html-api/wpHtmlProcessor-serialize.php:

• On trunk (63095): OK (1 test, 2 assertions)
• With only class-wp-html-tag-processor.php reverted to [62960]^, source otherwise untouched: fails with

  An attribute enqueued for removal was serialized as a value-less attribute.
  -'<div class="x">'
  +'<div onclick class="x">'
  

So it genuinely exercises what [62960] changed rather than passing vacuously.

### Timing

Tests only, no source changes, so this is safe during RC, but nothing breaks if it slips to 7.2 instead. Entirely the committers' call.

Background: I raised this serialization symptom on the ticket at comment:15 while the fix was still being chosen between four competing PRs, and offered the test to whichever branch was picked. #12757 landed without it, so I am submitting it here rather than letting it drop.

Thanks @irozum, and thanks in particular for reverting the tag processor and running it rather than reading it.

## On the assertStringContainsString

Not a deliberate hedge against ordering, and you were right to ask. I measured it: the output is deterministic, and a newly added attribute is emitted before the existing ones.

set id on class-only        <div id="new" class="x">
set id on two attrs         <div id="new" class="x" data-a="1">
overwrite existing class    <div class="y">
set id then remove class    <div id="new" data-a="1">

So the assertion can pin the whole string the way the removal assertion above it does, and it now does, in 891c513:

$this->assertSame(
        '<div id="new" class="x">',
        $processor->serialize_token(),
        'An attribute enqueued via set_attribute() was not serialized. A newly added attribute is emitted before the existing ones.'
);

The failure message names the ordering on purpose. Either assertion catches the original defect, since that one dropped the attribute outright, so this is about strength rather than correctness: the tighter form also pins where an added attribute lands, which nothing else in the file covers.

## Also rebased

The branch was 119 commits behind and class-wp-html-tag-processor.php has moved since, so I rebased onto current trunk rather than leave the result inferred from an old base. Tests_HtmlApi_WpHtmlProcessor_Serialize on trunk 4a8a86b: OK (133 tests, 269 assertions). phpcs clean on the file.

CI note so the one red mark is not read as this test failing.

891c513 is 108 success, 16 skipped, 1 cancelled. The cancelled one is PHP 7.4 / MySQL 8.0, and it is a job timeout rather than a test result:

........................ 30739 / 31011 ( 99%)
##[error]The operation was canceled.

Not one F or E in 30,739 tests before the runner was killed. It ran 14:23:01 to 14:43:21, exactly 20 minutes, which is the ceiling set in reusable-phpunit-tests-v3.yml:133:

timeout-minutes: ${{ inputs.coverage-report && 120 || inputs.php == '8.4' && 30 || 20 }}

That cell normally finishes with only a couple of minutes to spare. On trunk today the same job ran eight times at 13:02, 13:12, 13:20, 13:33, 13:34, 13:37, 13:44 and 18:20 minutes. A slow runner tips it over the line.

It is not specific to PHP 7.4 either: trunk run ​33160385316 lost PHP 8.5 / MySQL 8.0 multisite at 20:17 the same way, with no patch of mine anywhere near it.

I have no write access here, so I cannot re-run the job. Happy to push a no-op commit to retrigger if a committer would rather see the matrix fully green before looking at this, but the change under review is one assertion in one HTML API test file.