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.

Sign up for GitHub

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

Jump to bottom

Add/update Chromium data for Window API #6838

Merged
merged 2 commits into from
Oct 8, 2020
Merged

Add/update Chromium data for Window API #6838

merged 2 commits into from
Oct 8, 2020

Conversation

queengooborg
Copy link
Contributor

This PR updates the Chromium data for the Window API based upon a combination of manual testing and collector results to achieve more real data for the Window API. (Note: most Opera updates have been deferred to #6837.)

Tests used: https://mdn-bcd-collector.appspot.com/tests/api/Window

@github-actions github-actions bot added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Oct 5, 2020
@foolip foolip mentioned this pull request Oct 8, 2020
Open
foolip
foolip reviewed Oct 8, 2020
View reviewed changes
Copy link
Contributor

@foolip foolip left a comment

Choose a reason for hiding this comment

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

I didn't review, just spotted cancelAnimationFrame and left that comment.

api/Window.json
@@ -972,10 +972,10 @@
"mdn_url": "https://developer.mozilla.org/docs/Web/API/Window/cancelAnimationFrame",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you test for when the prefixed variants were added? They're included for requestAnimationFrame: https://developer.mozilla.org/en-US/docs/Web/API/Window/requestAnimationFrame

Tests:
http://mdn-bcd-collector.appspot.com/tests/api/Window/webkitCancelAnimationFrame
http://mdn-bcd-collector.appspot.com/tests/api/Window/webkitCancelRequestAnimationFrame (after foolip/mdn-bcd-collector#601)

foolip
foolip reviewed Oct 8, 2020
View reviewed changes
Copy link
Contributor

@foolip foolip left a comment

Choose a reason for hiding this comment

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

I've finished reviewing now. This is a big improvement, so I'll go ahead and back out the bits I'm not so sure about and merge what remains, to not risk getting conflicts and multiple roundtrips.

api/Window.json Outdated
@@ -1560,10 +1560,10 @@
"mdn_url": "https://developer.mozilla.org/docs/Web/API/Window/crypto",
"support": {
"chrome": {
"version_added": "37"
"version_added": "1"
Copy link
Contributor

Choose a reason for hiding this comment

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

This can't be right, window.crypto is newer than this. https://trac.webkit.org/changeset/78321/webkit is when it was added to WebKit, although there was something before that returning undefined with a "FIXME: Implement me" comment. I didn't check how that that was around for.

This also threw off results for Safari, so maybe we should do a custom test for window.crypto that ensures it exists and is an object at least?

Copy link
Contributor

Choose a reason for hiding this comment

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

#6825 (comment) the other place we talked about this.

api/Window.json Outdated
@@ -2180,10 +2180,10 @@
"mdn_url": "https://developer.mozilla.org/docs/Web/API/Window/external",
"support": {
"chrome": {
"version_added": true
"version_added": "1"
Copy link
Contributor

Choose a reason for hiding this comment

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

window.external is a bit special, I think that for a long time it wasn't defined using Web IDL a bit like console. Have you checked what kind of thing it is in Chrome 1, was it an object with methods on it? I'm worried it might be a similar situation to window.crypto above.

api/Window.json
},
{
"version_added": true,
"version_added": "1",
"alternative_name": "pageXOffset"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is api.Window.scrollX. I only noticed this now, but we also have a api.Window.pageXOffset entry, so these a alternative_name entries are weird, especially when both go back to ancient times. Can you clean that up in a separate PR?

@foolip foolip mentioned this pull request Oct 8, 2020
Merged
@foolip
Copy link
Contributor

foolip commented Oct 8, 2020

@vinyldarkscratch I'll merge this now, but to avoid ambiguity these are the things I'm asking for post-merging:

  • redoing the window.crypto and window.external changes that I reverted in a separate PR (or 2 if you like)
  • adding prefixed statements for webkitCancelAnimationFrame and webkitCancelRequestAnimationFrame. might also check if the webkitRequestAnimationFrame data is correct too.
  • getting rid of "alternative_name": "pageXOffset" and "alternative_name": "pageYOffset" statements under scrollX/scrollY if they're not adding beyond the pageXOffset/pageYOffset entries.

@foolip foolip merged commit 74b2e8c into mdn:master Oct 8, 2020
@queengooborg queengooborg deleted the api/Window-chrome branch October 8, 2020 22:14
foolip added a commit to foolip/browser-compat-data that referenced this pull request Jan 5, 2022
@foolip
Fix Chromium data for devicemotion event interfaces
be988c1 
This was all added in WebKit trunk 534.5 and 534.9:
https://trac.webkit.org/changeset/64845/webkit
https://trac.webkit.org/changeset/68252/webkit

It wasn't enabled then, but because it all went together, it can all be
assumed to match the ondevicemotion property being exposed. That was
confirmed to be Chrome 31 with this test:
http://mdn-bcd-collector.appspot.com/tests/api/Window/ondevicemotion

The source of Chrome 11 is here:
mdn#5834

The description says Chrome 31, so this must have been a copy-paste
error from the IE version.

Also update the Samsung Internet and WebView versions based on
conservative mirroring. It's possible this was supported earlier, but
the current version numbers also seem to be based on mirroring, not
testing:
mdn#6838
@foolip foolip mentioned this pull request Jan 5, 2022
Merged
queengooborg pushed a commit that referenced this pull request Jan 8, 2022
@foolip
Fix Chromium data for devicemotion event interfaces (#14445)
5309c7e 
This was all added in WebKit trunk 534.5 and 534.9:
https://trac.webkit.org/changeset/64845/webkit
https://trac.webkit.org/changeset/68252/webkit

It wasn't enabled then, but because it all went together, it can all be
assumed to match the ondevicemotion property being exposed. That was
confirmed to be Chrome 31 with this test:
http://mdn-bcd-collector.appspot.com/tests/api/Window/ondevicemotion

The source of Chrome 11 is here:
#5834

The description says Chrome 31, so this must have been a copy-paste
error from the IE version.

Also update the Samsung Internet and WebView versions based on
conservative mirroring. It's possible this was supported earlier, but
the current version numbers also seem to be based on mirroring, not
testing:
#6838
jpmedley pushed a commit that referenced this pull request Jan 10, 2022
@foolip @jpmedley
Fix Chromium data for devicemotion event interfaces (#14445)
b20c66c 
This was all added in WebKit trunk 534.5 and 534.9:
https://trac.webkit.org/changeset/64845/webkit
https://trac.webkit.org/changeset/68252/webkit

It wasn't enabled then, but because it all went together, it can all be
assumed to match the ondevicemotion property being exposed. That was
confirmed to be Chrome 31 with this test:
http://mdn-bcd-collector.appspot.com/tests/api/Window/ondevicemotion

The source of Chrome 11 is here:
#5834

The description says Chrome 31, so this must have been a copy-paste
error from the IE version.

Also update the Samsung Internet and WebView versions based on
conservative mirroring. It's possible this was supported earlier, but
the current version numbers also seem to be based on mirroring, not
testing:
#6838
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@foolip foolip foolip approved these changes

Assignees
No one assigned
Labels
data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@queengooborg @foolip