-
Notifications
You must be signed in to change notification settings - Fork 133
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
Add DocumentAndElementEventHandlers #413
Add DocumentAndElementEventHandlers #413
Conversation
Supported by Firefox and Edge (upcoming version) as specified. Supported by Chrome and Safari on Element (HTML defines them on HTMLElement) Tests: web-platform-tests/wpt#10249
Do I need to make something like:
that lists all event handler attributes in DocumentAndElementEventHandlers and add them to all 44 places in SVG? This seems to defy the point in mixins if we have to maintain every content attribute that reflects a IDL attribute in a mixin that is defined in another spec. |
Good point. Technically, the mixin only defines the DOM interface properties. It doesn't add the attributes to the element model. So the attributes do need to be defined separately. That said, the text in the Event Handlers section in the interactivity chapter is pretty open-ended, automatically including any new event types:
So maybe a better strategy would be to get rid of the list of explicit attributes for the "global event attributes" category, and instead make it link to that section. But I'm not sure where in the build process we need to change something to make that happen. |
Until we work out the build process, I'll just add DocumentAndElementEventHandlers every where that GlobalEventHandlers is included for now. As it is for cut/copy/paste it probably won't change too much, unless onbeforecut/copy/paste are standardized (which are supported by all browsers) |
master/changes.html
Outdated
@@ -178,6 +178,7 @@ <h3 id="types">Basic Data Types and Interfaces chapter (owner: BogdanBrinza)</h3 | |||
<li><a>SVGUnitTypes</a> is no longer NoInterfaceObject. <a href="https://github.com/w3c/svgwg/issues/291">Github #291</a>.</li> | |||
<li>Clarified that <a>SVGElement</a>.<a href="types.html#__svg__SVGElement__className">className</a> | |||
overrides <a>Element</a>.className. Not normative, <a href="https://github.com/w3c/svgwg/issues/298">Github #298</a>.</li> | |||
<li>Made <a>SVGElement</a> include the <a>DocumentAndElementEventHandlers</a> interface from HTML.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this require a "changed since last publication" div? Same for the above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this already is in the changed-since-cr1 or has there been another CR and this and the previous change needs to go into a changed-since-cr2 div?
@dirkschulze / @AmeliaBR : this should be ready for final review I believe. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though I gave lgtm already, sorry.
Fixes #395 (any additional members should have their own issue)
Supported by Firefox and Edge (upcoming version) as specified. Supported by Chrome and Safari on Element (HTML defines them on HTMLElement so Safari/Chrome have them one level too high)
Tests: web-platform-tests/wpt#10249