Single event entries for TextTrack APIs#13887
Conversation
|
Resolved the merge conflict (caused by #13936) and added Webview Android versions from that PR to the eventName_event entries. |
| "version_added": "10" | ||
| }, | ||
| "opera": { | ||
| "version_added": "≤12.1" |
There was a problem hiding this comment.
On second review I see that the "oncuechange" and "cuechange_event" data doesn't match for Opera or Safari. There's probably a lot of small differences. Can you list what they are for ease of review?
Such differences are really the only thing worth looking at when reviewing these PRs, so maybe it's worth writing a script to list the differences and generate a boilerplate comment for each PR?
|
Here are the differences: api.Texttrack.cuechange_event api.Texttrack.oncuechange api.TextTrackList.removetrack_event api.TextTrackList.onremovetrack |
|
My assumption is that the onremovetrack and the oncuechange entries are more correct because these were updated by the collector. |
|
Yes, I'd take the For "cuechange_event", I think this would be a good guess that's fairly consistent with the other data: |
|
Another thing here is api.GlobalEventHandlers.oncuechange. browser-compat-data/api/GlobalEventHandlers.json Line 1016 in 44310a2 My guess would be to remove this data as it is captured in api.TextTrack.cuechange_event and api.HTMLTrackElement.cuechange_event. Not sure where to redirect https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/oncuechange to. Either to https://developer.mozilla.org/en-US/docs/Web/API/TextTrack/cuechange_event or to https://developer.mozilla.org/en-US/docs/Web/API/HTMLTrackElement/cuechange_event |
42fd335 to
81c18ec
Compare
81c18ec to
24b1743
Compare
I redirected to TextTrack. Appreciate a look at the content change, too: mdn/content#12684 Will remove api.GlobalEventHandlers.oncuechange here then, too. |
|
I made the mistake of checking if the two remaining "cuechange_event" entries on A trivial issue is that a link to https://html.spec.whatwg.org/multipage/webappapis.html#handler-oncuechange is missing. https://trac.webkit.org/changeset/100064/webkit introduced the two events in WebKit, so the versions for Safari should match, but they don't. The data for I doubt the data for Opera is correct, and would suggest setting it to "12" like most of There are also differences in the IE and Edge data that I haven't researched, but I wouldn't be surprised if it's wrong. Also, should we capture that the |
|
Thanks for your detailed view. Unfortunately I don't know all the answers.
Done.
Done.
I've added a range to show partial support. The wording is per our guideline: https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines.md#dom-events-eventname_event For the issues with IE support and Safari versions, I don't know how to resolve the issues you found. You could file it as a new issue and we land this to continue refactoring other event data? |
| { | ||
| "version_added": "6", | ||
| "partial_implementation": true, | ||
| "notes": "The <code>oncuechange</code> event handler property is not supported." |
There was a problem hiding this comment.
We'll need a note like this for Chrome and friends too. Using http://software.hixie.ch/utilities/js/live-dom-viewer/?saved=10032 I found that it was added in Chrome 32.
|
If this is approved, it can be merged. The content work got merged in mdn/content#12688 and mdn/content#12684 |
foolip
left a comment
There was a problem hiding this comment.
There's still a mix of Safari/iOS versions that must be wrong, with 6/6 in some places and 6/7 in others. But that problem exists outside of the updated data, so let's leave that for another day.
Per the new guideline: https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines.md#dom-events-eventname_event
Content work to be done.