Repository navigation
Finalise spec mechanism for event handlers #520
Description
As of #457 the spec uses the HTML "event handler content attribute" concept. Anne's feedback was that that was ambiguous and we should instead generate a fixed list to check inside of. This issue tracks generating that or an alternative.
cc @koto as spec editor you might know how best to deal with this.
Activity
koto commentedon Jun 28, 2024
@otherdaniel how does Sanitizer deal with this? From memory, the challenge was that there was no centralized and specced list of event handlers we could refer to.
otherdaniel commentedon Jun 28, 2024
Yes, there isn't a spec list of event handlers. Sanitizer has it a bit easier, since it's based on an allow-list: We (manually) write a list of allowed attributes, and we maintain a list of all attributes. Sanitizer doesn't have to decide why an attribute (or element) isn't in the allow-list, e.g. whether it's an event handler or not.
There was some discussion of adding some sort of spec-level 'tags' to any known element or attribute that could then form a basis for these lists; but this mechanism doesn't currently exist. Until it does, it'll be manually maintained lists.
lukewarlow commentedon Jun 28, 2024
I'm happy to spend some time and put together a list.
But from a spec maintainer and implementation point of view a block list of element event handler attributes seems far from ideal. Any time someone adds a new one they have to remember to adjust that list. Rather than current implementations which just use the IDL (which I'm realistically gonna use as the basis for my list). Especially tricky if the event handler is specified in a random wicg spec or something like that. Also if there's non standard events browsers have they should also be protected but won't be specced.
That being said I'm happy to defer to others on whether that's okay in practice.
koto commentedon Jun 28, 2024
So, rephrasing, the choice is to be either ambiguous and refer to other spec concepts (event handler content attribute), or explicit, and maintain a separate list (here, or in HTML?). I actually think the first choice is better, from normative perspective.
otherdaniel commentedon Jun 28, 2024
From an implementation point of view, there's two easy ways:
EventHandler,OnBeforeUnloadEventHandler, orOnErrorEventHandler.But I don't think this is how we can spec things.
lukewarlow commentedon Jun 28, 2024
Unfortunately custom attributes would fall foul of that and apparently chrome's had issues with it.
otherdaniel commentedon Jun 28, 2024
What I meant is, whether it's a spec-defined attribute and starts with "on". Getting a list of attributes described in the spec and filtering for the "on" prefix gets you the list of event handler attributes, as they've been consistent with the naming.
annevk commentedon Jul 1, 2024
"Specification-defined attribute" is also not a concept that exists though, currently.
fred-wang commentedon Oct 22, 2024
Skimming over existing WPT tests, it seems all the event handler attributes used in tests are coming from GlobalEventHandlers. Conversely, all event handler attributes from
GlobalEventHandlersare covered bytrusted-types-event-handlers.html(which tries all the names of a div's proto starting with 'on' of at least 3 characters)Trying to implement
getAttributeTypein Firefox, it's not clear from the current text of the spec whether the attributes below should also be covered:HTMLBodyElementandHTMLFrameSetElement, some of them also supported by the<svg>element.onencryptedandonwaitingforkeyonHTMLMediaElementfrom the Encrypted Media Extensions.onbegin,onendandonrepeat).Add WPT tests for getAttributeType with event handler content attribu…
Bug 1917783 - Add WPT tests for getAttributeType with event handler c…
Add WPT tests for getAttributeType with event handler content attribu…
Add WPT tests for getAttributeType with event handler content attribu…
1 remaining item
Update Trusted Types content attributes test (#49473)
Bug 1934650 [wpt PR 49473] - Update Trusted Types content attributes …
Bug 1934650 [wpt PR 49473] - Update Trusted Types content attributes …
Bug 1934650 [wpt PR 49473] - Update Trusted Types content attributes …
Bug 1934650 [wpt PR 49473] - Update Trusted Types content attributes …
fred-wang commentedon Jan 14, 2025
Tests trying to be a bit more complete have been added since my previous comment:
https://wpt.fyi/results/trusted-types/TrustedTypePolicyFactory-getAttributeType-event-handler-content-attributes.tentative.html
https://wpt.fyi/results/trusted-types/set-event-handlers-content-attributes.tentative.html
Note that the list of content event attribute (and associated interface) are enumerated using this support file:
https://github.com/web-platform-tests/wpt/blob/master/trusted-types/support/event-handler-attributes.mjs
See also #573
annevk commentedon Jan 14, 2025
@fred-wang that's great to see! Could you also add tests for event handlers that are not event handler content attributes? E.g.,
onreadystatechange,onopen, andondoesnotexist.cc @lukewarlow
Add trusted types tests for setAttribute that are not sinks.
fred-wang commentedon Jan 27, 2025
@annevk @lukewarlow finally open the PR web-platform-tests/wpt#50295
Add trusted types tests for setAttribute that are not sinks. (#50295)
Bug 1943976 [wpt PR 50295] - Add trusted types tests for setAttribute…
fred-wang commentedon Feb 3, 2025
@annevk @lukewarlow One thing that I have been wondering for a while, is whether we should make the "Get Trusted Type data for attribute" a bit more restrictive and only test for "event handler content attribute" if the element is in the HTML, SVG or MathML namespace?
annevk commentedon Feb 25, 2025
@fred-wang that sounds good to me given that
GlobalEventHandlersis not included byElementat this point.Bug 1943976 [wpt PR 50295] - Add trusted types tests for setAttribute…
Bug 1917783 - Add WPT tests for getAttributeType with event handler c…