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 GlobalEventHandlers.beforexrselect_event #16026

Closed
wants to merge 1 commit into from

Conversation

Elchi3
Copy link
Member

@Elchi3 Elchi3 commented Apr 26, 2022

Companion PR for mdn/content#15376

Discussion: #7545 (comment) and ff.

@github-actions github-actions bot added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Apr 26, 2022
@Elchi3
Copy link
Member Author

Elchi3 commented Apr 28, 2022

fwiw, the alternative would be to add this under the specific interfaces:

api.Document.beforexrselect_event
api.Window.beforexrselect_event
api.HTMLElement.beforexrselect_event
api.SVGElement.beforexrselect_event
api.MathMLElement.beforexrselect_event

@foolip
Copy link
Collaborator

foolip commented May 4, 2022

As argued in mdn/content#15376 (comment), I think this should go under api.Element.beforexselect_event.

@Elchi3
Copy link
Member Author

Elchi3 commented May 5, 2022

@saschanaz would api.Element.beforexrselect_event work for you even if the IDLs don't install it under Element?

@Elchi3
Copy link
Member Author

Elchi3 commented May 5, 2022

Also @queengooborg, would this work with the collector? Would you remap this to Element somehow?

@queengooborg
Copy link
Collaborator

queengooborg commented May 5, 2022

Yes, this would be remapped to the Element API by the collector already, as the collector demixes all mixins before generating tests!

Edit: this will also map to Document, HTMLElement, and anything else that "implements GlobalEventHandlers;" in IDL!

@saschanaz
Copy link
Contributor

saschanaz commented May 5, 2022

Also @queengooborg, would this work with the collector? Would you remap this to Element somehow?

My current logic check two places: The original mixin or each interface. It would be hard to find Document.onbeforexrselect when it's in Element since it's not an ancestor of Document. (I should fix the logic even if it was an ancestor though)

But theoretically it can still be somehow tracked as it has GlobalEventHandlers anyway, I should gather all the interfaces and check the support data exists anywhere, although that's kinda weird since there may or may not exist multiple support data or not.

@foolip
Copy link
Collaborator

foolip commented May 5, 2022

Yes, this would be remapped to the Element API by the collector already, as the collector demixes all mixins before generating tests!

For anything in GlobalEventHandlers, the collector will generate the tests for HTMLElement+SVGElement+MathMLElement, but not Element. The reason there's also a test on Element in the collector is https://github.com/foolip/mdn-bcd-collector/blob/main/custom-idl/dom-overlays.idl, but that's the exception, not the rule.

The problem we have here is that GlobalEventHandlers adds event handler attributes everywhere, but in documentation we want to be specific about where events fire.

The *_event entries might be on both a more general interface like Element because they can fire, or at least bubble through, any kind of Element. They might also be on a more specific interface like HTMLVideoElement because they only fire on that specific kind of element.

I think that a mapping would have treat all kinds of elements as one class, all kind of documents as one class, and also know whether a specific event bubbles. The bubbling information is currently not machine readable anywhere, but maybe a decent approximation is to guess that all events bubble.

@github-actions
Copy link

github-actions bot commented Jun 3, 2022

This pull request has merge conflicts that must be resolved before it can be merged.

@queengooborg
Copy link
Collaborator

Thanks @Elchi3! However, we ended up deciding to demix GlobalEventHandlers as it's the only mixin left and it's not fully representative of what events fire on what interfaces.

@Elchi3 Elchi3 deleted the webxr-beforexrselect branch August 16, 2022 16:49
@Elchi3
Copy link
Member Author

Elchi3 commented Aug 16, 2022

Thanks @Elchi3! However, we ended up deciding to demix GlobalEventHandlers as it's the only mixin left and it's not fully representative of what events fire on what interfaces.

okay, thanks! How should I add beforexrselect to BCD now?

fwiw, the alternative would be to add this under the specific interfaces:

api.Document.beforexrselect_event
api.Window.beforexrselect_event
api.HTMLElement.beforexrselect_event
api.SVGElement.beforexrselect_event
api.MathMLElement.beforexrselect_event

^ like this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants