-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
SourceBuffer(List) / MediaSource: update event entries #13924
Conversation
Content PR merged. This is ready. |
api/SourceBuffer.json
Outdated
], | ||
"support": { | ||
"chrome": { | ||
"version_added": "53" |
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.
Chrome 53 is when onabort
and lots of other event handler properties were added:
https://storage.googleapis.com/chromium-find-releases-static/000.html#00003ed19f12e054784ee59b95c863b380361ef4
I would suggest also including MediaSource
in this PR, to sort it all out at once.
Unfortunately, we'll have to find the versions when each event was supported.
It also looks like it's the same situation for Edge. Firefox and Safari match the parent feature version though, so those don't need any research.
@Elchi3 it will be a lot of work to sort this out, and really any time that the event's version doesn't match the parent feature I'd be suspicious and want to check if it's yet another case of the event handler property being missing originally. |
@Elchi3 so my question is, how do you want to handle this? Doing the research as part of PR review is going to slow things down, but can you maintain a list of likely incorrect data in an issue for later review? |
I've added MediaSource, so it is all together in this PR now. For all events, the question is: Is it Chrome 53 or Chrome 31 (Chrome 25 with webkit)? SourceBuffer.abort_event It currently states the later versions. |
@Elchi3 the event handler properties were added in Chromium 53: So we'll need The events themselves look complicated. Some of the events themselves were originally prefixed, with https://trac.webkit.org/changeset/123522/webkit being an example of this. (But note how no code was added to actually fire "webkitaddsourcebuffer" or "webkitremovesourcebuffer" there.) @Elchi3 would you like to capture the prefixed events so that we have data going back to when the interfaces were added? This will require digging in WebKit source for each event. I can show you how I do it if you'd like. |
I'm happy to add ranges like these:
I have no idea how to find $versionnumber. Pointers appreciated.
I don't know how to find the complete history here and I'm not sure it is worth to go back in time so much? |
OK, so let's ignore the prefixed events and try to get reasonable versions for when the unprefixed events were supported. |
Co-authored-by: Philip Jägenstedt <[email protected]>
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.
Found some more things wrong when trying to do "final" review.
Note that the IE data is also arguably wrong, I'm pretty sure it was also missing the event handler properties. I won't insist on adding that though.
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.
I started to review again and found more things wrong, but at this point I'm just going to look away :)
Ha, thanks for merging! This has been quite an advantage in compat archeology. |
For future archeologists: we still have a mix of 41 and 42 versions for Firefox Android, which is almost certainty wrong. |
Update for new event structure.
Content update: mdn/content#11176