Interactivity API: Preserve colons and semicolons in existing style attributes - #12733
Interactivity API: Preserve colons and semicolons in existing style attributes#12733itzmekhokan wants to merge 1 commit into
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. |
There was a problem hiding this comment.
Pull request overview
This PR fixes WP_Interactivity_API::merge_style_property() so it no longer corrupts existing style attribute content when processing data-wp-style--* directives, specifically for CSS values containing colons and for segments that contain no colon.
Changes:
- Split style declarations on the first colon only (
explode( ':', ..., 2 )) to preserve colons inside valid CSS values (e.g.,url(https://...), data URIs). - Add a guard to preserve colon-less segments verbatim to avoid PHP diagnostics and to allow semicolon-split tails (from quoted strings/
url()) to re-join correctly. - Add PHPUnit coverage for colon/semicolon cases, colon-less segments, and an end-to-end
data-wp-stylescenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/wp-includes/interactivity-api/class-wp-interactivity-api.php |
Fixes parsing in merge_style_property() to preserve colon-containing values and safely handle colon-less segments. |
tests/phpunit/tests/interactivity-api/wpInteractivityAPI-wp-style.php |
Adds tests covering colon/semicolon preservation, colon-less segments, and directive behavior with URL-containing styles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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. |
…ttributes. `merge_style_property()` re-serializes the whole `style` attribute each time a `data-wp-style--*` directive is processed. It split each declaration with `explode( ':', $style_assignment )` and no limit, truncating any value that contained a colon, so `background-image:url(https://example.com/a.png)` became `background-image:url(https`. Segments containing no colon at all reached `list()` and raised `Undefined array key 1` plus a `trim()` deprecation notice. Splits on the first colon only, and preserves a segment that contains none verbatim, so that a value split by a semicolon inside a quoted string or a `url()` is restored when the segments are re-joined. Props khokansardar, tkay01, sabernhardt. Trac ticket: https://core.trac.wordpress.org/ticket/63899
ff790dc to
cd457f5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/wp-includes/interactivity-api/class-wp-interactivity-api.php:1176
- The docblock note references “See #65738.”, but this PR (and the added tests) are scoped to Trac ticket 63899. If this note is meant to point to the current issue, the ticket number should match to avoid confusion when backporting or searching history.
* Declarations are separated on semicolons without regard for quoted strings or
* `url()` values, so the property being replaced is not recognized when its own
* value contains a semicolon. Resolving that requires tokenizing the declaration
* list rather than splitting it. See #65738.
`){kind=link}
WP_Interactivity_API::merge_style_property()re-serializes the entirestyleattribute every time adata-wp-style--*directive is processed. Two defects sit in that loop, both on the same line.1. Values containing a colon are truncated.
explode( ':', $style_assignment )is called without a limit, so everything after the first colon is discarded. This corrupts valid CSS silently, with no warning:The background image is destroyed on the front end. Any
url()with a scheme triggers it, as does any custom property holding one.2. Segments containing no colon raise PHP diagnostics. They yield no
$value, solist()raisesUndefined array key 1and the subsequenttrim( null )is deprecated on PHP 8.1+. This is the half originally reported on the ticket.There is a third, related symptom: declarations are split on
;before anything knows where strings begin and end, so a semicolon inside a quoted string or aurl()cuts a value in two.font-family:"Foo;Bar",serifandurl(data:image/gif;base64,…)both split, and the tail then hits the colon-less path above and picks up a stray:.Changes
explode( ':', $style_assignment, 2 ), so only the first colon separates a property name from its value.The guard preserves such segments verbatim and untrimmed rather than dropping them. This also repairs the semicolon cases: because segments are re-joined with
;, preserving the tail restoresfont-family:"Foo;Bar",serifandurl(data:image/gif;base64,…)intact. Dropping them would emitfont-family:"Foo— an unterminated string, which is worse than the original bug. Leaving them untrimmed matters too, or the space in"Foo; Bar"is lost.Trac ticket: https://core.trac.wordpress.org/ticket/63899
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Ticket analysis, tests cases and writing PR description. All changes were reviewed and validated by me.
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.