-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Correct Safari data for various Media APIs #7427
Conversation
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.
We seem to have some ghost features here that should be removed I think
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.
There are a fair number of apparent test issues here. It might be a good idea to split out the changes that are added claimed support from those that are removing claimed support, since the latter requires much more scrutiny and will hold up the former.
api/MediaStreamAudioSourceNode.json
Outdated
@@ -29,10 +29,10 @@ | |||
"version_added": "14" | |||
}, | |||
"safari": { | |||
"version_added": "6" | |||
"version_added": false |
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 suspect this change is incorrect and MediaStreamAudioSourceNode
is supported. http://mdn-bcd-collector.appspot.com/tests/api/MediaStreamAudioSourceNode return false in many places making it hard to interpret the results.
@vinyldarkscratch can you check out why we're seeing a false result here?
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.
@vinyldarkscratch this conversation was resolved but it looks like the change still remains, so reopening.
@vinyldarkscratch should any of the changes in api/MediaStreamAudioSourceNode.json remain, or should they be reverted? |
It was introduced here together with setLiveSeekableRanges: https://trac.webkit.org/changeset/206146/webkit https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/Configurations/Version.xcconfig?rev=206146 Testing https://mdn-bcd-collector.appspot.com/tests/api/MediaSource in Safari 10 and 10.1 in Sauce Labs confirms they were added in 10.1. The data came from #7427 which did come from the collector, but even the results at the time support 10.1 as the correct version: https://raw.githubusercontent.com/foolip/mdn-bcd-results/327933b8a3a402bbb6e880569a2dff104df31698/1.1.3-safari-10.0.1-mac-os-10.11.6-daabd96f93.json https://raw.githubusercontent.com/foolip/mdn-bcd-results/327933b8a3a402bbb6e880569a2dff104df31698/1.1.3-safari-10.1.2-mac-os-10.12.6-142e4c09c5.json Possibly the updater script is buggy or the results were manually edited.
It was introduced here together with setLiveSeekableRanges: https://trac.webkit.org/changeset/206146/webkit https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/Configurations/Version.xcconfig?rev=206146 Testing https://mdn-bcd-collector.appspot.com/tests/api/MediaSource in Safari 10 and 10.1 in Sauce Labs confirms they were added in 10.1. The data came from #7427 which did come from the collector, but even the results at the time support 10.1 as the correct version: https://raw.githubusercontent.com/foolip/mdn-bcd-results/327933b8a3a402bbb6e880569a2dff104df31698/1.1.3-safari-10.0.1-mac-os-10.11.6-daabd96f93.json https://raw.githubusercontent.com/foolip/mdn-bcd-results/327933b8a3a402bbb6e880569a2dff104df31698/1.1.3-safari-10.1.2-mac-os-10.12.6-142e4c09c5.json Possibly the updater script is buggy or the results were manually edited.
This PR corrects the Safari data for various Media APIs using results from the mdn-bcd-collector project.