-
Notifications
You must be signed in to change notification settings - Fork 75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Finalise spec mechanism for event handlers #520
Comments
@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. |
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. |
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. |
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. |
From an implementation point of view, there's two easy ways:
But I don't think this is how we can spec things. |
Unfortunately custom attributes would fall foul of that and apparently chrome's had issues with it. |
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. |
"Specification-defined attribute" is also not a concept that exists though, currently. |
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 Trying to implement
|
…tes. The spec is not really clear, for now add a tentative test with the cases detected while implementing the method in Firefox. For details, see w3c/trusted-types#520 Differential Revision: https://phabricator.services.mozilla.com/D226442 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1917783 gecko-commit: 474afb5197ff041a421591d890dd339a2b23fe08 gecko-reviewers: smaug
…ontent attributes. r=smaug The spec is not really clear, for now add a tentative test with the cases detected while implementing the method in Firefox. For details, see w3c/trusted-types#520 Differential Revision: https://phabricator.services.mozilla.com/D226442
…tes. The spec is not really clear, for now add a tentative test with the cases detected while implementing the method in Firefox. For details, see w3c/trusted-types#520 Differential Revision: https://phabricator.services.mozilla.com/D226442 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1917783 gecko-commit: 474afb5197ff041a421591d890dd339a2b23fe08 gecko-reviewers: smaug
…tes. The spec is not really clear, for now add a tentative test with the cases detected while implementing the method in Firefox. For details, see w3c/trusted-types#520 Differential Revision: https://phabricator.services.mozilla.com/D226442 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1917783 gecko-commit: 474afb5197ff041a421591d890dd339a2b23fe08 gecko-reviewers: smaug
…ontent attributes. r=smaug The spec is not really clear, for now add a tentative test with the cases detected while implementing the method in Firefox. For details, see w3c/trusted-types#520 Differential Revision: https://phabricator.services.mozilla.com/D226442
…ontent attributes. r=smaug The spec is not really clear, for now add a tentative test with the cases detected while implementing the method in Firefox. For details, see w3c/trusted-types#520 Differential Revision: https://phabricator.services.mozilla.com/D226442
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.
The text was updated successfully, but these errors were encountered: