Emit valid values for aria-pressed on the Customizer device buttons - #13046
Emit valid values for aria-pressed on the Customizer device buttons#13046westonruter wants to merge 1 commit into
Conversation
The responsive-preview buttons in the Customizer rendered aria-pressed via esc_attr( $active ), where $active is a bool. PHP casts true to '1' and false to '', so the desktop button emitted aria-pressed="1" and the tablet and mobile buttons emitted aria-pressed="". Neither is a valid value. aria-pressed is a tristate attribute accepting only "true", "false", "mixed", or "undefined", and an invalid or empty value is treated as "undefined" — meaning the button is not exposed as a toggle at all. All three buttons were therefore announced without their pressed state on initial render, until the first interaction caused controls.js to rewrite the attribute via jQuery, which stringifies the bool correctly to "true"/"false". Emit the literals directly so the server render agrees with what the JavaScript later writes. This matches the existing pattern used for aria-expanded in wp-admin/includes/template.php. Also makes the sentinel explicit for the Site Icon button's data-state attribute, which relied on the same bool-to-string cast to produce the '1' and '' values that site-icon.js compares against and writes back. Behaviour is unchanged there; the contract is now stated rather than implied. Regenerating the baselines drops the last two esc_*() entries from tests/phpstan/baselines/argument.type.neon, which no longer has any. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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. |
While I do see Anyways, the fix makes totally sense. Good catch. |
@afercia no, I don't think you're missing anything. This is a minor correctness fix which probably won't have any a11y impact. Thank you for reviewing. |
The responsive-preview buttons in the Customizer emit invalid values for
aria-pressed, so none of the three is exposed to assistive technology as a toggle button on initial render.The defect
wp-admin/customize.phprendered the attribute throughesc_attr()with a bool:PHP casts
trueto'1'andfalseto the empty string. Since only the Desktop device carries'default' => true, the markup came out as:aria-pressedis a tristate attribute whose only valid values aretrue,false,mixed, andundefined. Both1and the empty string are invalid, and an invalid or empty value is treated asundefined, which means the element is not exposed as a toggle button at all. So on page load the pressed state of all three buttons was unavailable to screen reader users, and the currently selected preview size was not announced.The state does become correct after the first interaction, because
customize-controls.jssets it via jQuery:jQuery stringifies those to
"false"and"true", which are valid. So the JavaScript was always right and the server render disagreed with it — the markup only repaired itself once the user clicked something.The fix
Emit the literals directly, so the initial render agrees with what the JavaScript later writes:
No escaping is needed since both values are literals. This matches the existing pattern for
aria-expandedinwp-admin/includes/template.php, which assigns'true'/'false'as strings.Every
aria-pressed,aria-expanded,aria-selected,aria-checked,aria-disabled,aria-current, andaria-invalidvalue rendered from PHP acrosssrc/wp-admin,src/wp-includes, andsrc/wp-content/themeswas checked. This was the only one not already a valid literal.Also included
wp-admin/options-general.phpused the same bool-to-string cast for the Site Icon button'sdata-state:That one is not a defect.
site-icon.jscompares against the literal'1'and writes back'1'/'', so the cast happened to produce exactly the sentinels the script expects. It is changed tohas_site_icon() ? '1' : ''purely to state that contract rather than leave it resting on PHP's cast rules. Behaviour is identical.It is included here because these were the only two places in core passing a bool to an escaping function. Commit r63296 widened the
esc_*()annotations tostring|int|floatand deliberately excludedboolso that both call sites stayed visible rather than being silently permitted by the signature; resolving them empties the last of theesc_*()entries fromtests/phpstan/baselines/argument.type.neon.Trac ticket: Core-65817
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Identifying the invalid attribute values, sweeping core for other occurrences, the fix itself, and drafting this description. Reviewed 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.