Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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. |
751b1b2 to
891c513
Compare
|
Thanks @irozum, and thanks in particular for reverting the tag processor and running it rather than reading it. On the
|
|
CI note so the one red mark is not read as this test failing.
Not one 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 It is not specific to PHP 7.4 either: trunk run 33160385316 lost 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. |
891c513 to
fec52b2
Compare
…updates. [62960] made get_attribute_names_with_prefix() respect enqueued updates, but its tests only cover WP_HTML_Tag_Processor. WP_HTML_Processor::serialize_token() consumed the same stale name list while reading values through get_attribute(), so a removed attribute was re-emitted as a value-less attribute and an added one was dropped. That path gained no coverage. Fails at [62960]^ with '<div onclick class="x">', passes on trunk. Both assertions pin the full serialized output. A newly added attribute is emitted before the existing ones, so the set_attribute() case is deterministic and does not need a substring match (per @irozum's review). Rebased onto trunk. See #64567.
19c48db to
b08dd68
Compare
Trac ticket: https://core.trac.wordpress.org/ticket/64567
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 inwpHtmlTagProcessor.php.The bug had a second surface.
WP_HTML_Processor::serialize_token()iterates the names fromget_attribute_names_with_prefix( '' )but reads each value throughget_attribute(), so it consumed exactly the stale list the fix corrected. Before [62960] that meant:remove_attribute( 'onclick' )thenserialize_token()re-emitted the attribute as a value-less one:<div onclick class="x">set_attribute( 'id', 'new' )thenserialize_token()dropped the addition entirelyBoth are fixed by [62960], with no change to
serialize_token()itself. Neither is covered.This adds one test to
wpHtmlProcessor-serialize.phppinning 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:63095):OK (1 test, 2 assertions)class-wp-html-tag-processor.phpreverted to[62960]^, source otherwise untouched: fails withSo 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.
Use of AI Tools
AI assistance: Yes
Tool(s): Antigravity IDE
Model(s): Gemini 2.5 Pro
Used for: Code analysis, conflict resolution, test suite updates, and patch verification. All changes were manually reviewed and tested locally.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.