Skip to content
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

Merged
merged 3 commits into from
May 1, 2018

Conversation

dstorey
Copy link
Member

@dstorey dstorey commented Apr 12, 2018

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

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
@dstorey
Copy link
Member Author

dstorey commented Apr 12, 2018

Do I need to make something like:

<attributecategory
      name='document and element event'

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.

@AmeliaBR
Copy link
Contributor

Do I need to make something ... that lists all event handler attributes

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:

event attribute
An event attribute always has a name that starts with "on" followed by the name of the event for which it is intended. It specifies some script to run when the event of the given type is dispatched to the element on which the attribute is specified.

For every event type that the user agent supports, SVG supports that as an event attribute, following the same requirements as for event handler content attributes [HTML]. The event attributes are available on all SVG elements.

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.

@dstorey
Copy link
Member Author

dstorey commented Apr 19, 2018

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)

@@ -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>
Copy link
Contributor

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?

Copy link
Member Author

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?

@dstorey
Copy link
Member Author

dstorey commented Apr 20, 2018

@dirkschulze / @AmeliaBR : this should be ready for final review I believe.

Copy link
Contributor

@dirkschulze dirkschulze left a 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.

@dstorey dstorey merged commit da0cc34 into w3c:master May 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants