Skip to content

Conversation

@Elchi3
Copy link
Member

@Elchi3 Elchi3 commented Nov 26, 2023

Unfortunately, I see no more context for 8f23515. There was no pull request...

I'm suggesting to add event collections back but ignore everything that has to do with GlobalEventHandlers and WindowEventHandlers for now.

(Also, I know Serial events can be ignored due to bubbling to SerialPort. Added that exception in as well.)

queengooborg
queengooborg previously approved these changes Nov 26, 2023
@queengooborg
Copy link
Member

queengooborg commented Nov 26, 2023

Unfortunately, I see no more context for 8f23515. There was no pull request...

This change was discussed during the BCD call on November 30, 2022. The meeting notes state why they were disabled:

(Vinyl) Should we add event features for events that bubble up to interface, but interface has event handler properties? ([#18287](https://github.com/mdn/browser-compat-data/pull/18287), [#18288](https://github.com/mdn/browser-compat-data/pull/18288))
Philip: we removed a bunch of things re event handlers that were not needed
Philip: Some events that do bubble up were removed. Collector keeps adding entries back
Philip: BCD is now fairly consistent in that events show up where they happen. 
Vinyl: makes sense, no strong opinion either way. 
Philip: If we avoid having the collector add those events back, would that solve the problem?
Vinyl: Yes, seems that way. I can make changes so it’s not recreated on particular interfaces. Make collector disregard events in IDLs completely. Can handle things as custom implementations as needed

That's why #133 was proposed instead.

@Elchi3
Copy link
Member Author

Elchi3 commented Nov 26, 2023

I know the Global/WindowEventHandlers need a mapping before we can enable these again. We definitely don't want the collector to add these entries all over the place and that's why I skipped them in this PR.

I also added Serial events to be ignored but there are surely more events that bubble which we want to ignore. I think we should probably have better ignore code than what I did for Serial here.

I'm thinking though that enabling this back on should allow us to test support for legit events again and discover new legit events? Seems like that is totally turned off now, right?

@Elchi3
Copy link
Member Author

Elchi3 commented Nov 27, 2023

Main: 4657/15217 (30.60%) entries missing from collector that are in bcd
This branch: 4448/15217 (29.23%) entries missing from collector that are in bcd

Copy link
Member

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I see that this doesn't add any new features either (by running npm run find-missing-features bcd-from-collector), so let's merge this.

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