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.

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

Update compat data for PromiseRejectionEvent #3007

Merged
merged 2 commits into from
May 1, 2019

Conversation

TrevorBurnham
Copy link
Contributor

Follow-up for #2100

Currently, the MDN docs indicate that PromiseRejectionEvent and the associated window.onunhandledrejection and window.onrejectionhandled properties are Chrome-only. I believe the CanIUse data for these events are correct: https://caniuse.com/#feat=unhandledrejection

Also, I believe this same data should appear on https://developer.mozilla.org/en-US/docs/Web/Events/unhandledrejection#Browser_compatibility.

@Elchi3 Elchi3 assigned Elchi3 and unassigned Elchi3 Oct 25, 2018
@Elchi3 Elchi3 added the data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Oct 25, 2018
@ddbeck ddbeck self-requested a review December 21, 2018 17:15
ddbeck
ddbeck previously requested changes Jan 7, 2019
Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Hi @TrevorBurnham, thank you for this PR and welcome to BCD. A few changes to suggest on this one—we can get the data accuracy a little better I think—but this is a great start. Thank you again!

api/PromiseRejectionEvent.json Outdated Show resolved Hide resolved
api/PromiseRejectionEvent.json Outdated Show resolved Hide resolved
api/PromiseRejectionEvent.json Outdated Show resolved Hide resolved
api/PromiseRejectionEvent.json Outdated Show resolved Hide resolved
api/PromiseRejectionEvent.json Outdated Show resolved Hide resolved
api/WindowEventHandlers.json Outdated Show resolved Hide resolved
api/WindowEventHandlers.json Outdated Show resolved Hide resolved
api/WindowEventHandlers.json Outdated Show resolved Hide resolved
api/WindowEventHandlers.json Outdated Show resolved Hide resolved
api/PromiseRejectionEvent.json Outdated Show resolved Hide resolved
@TrevorBurnham
Copy link
Contributor Author

TrevorBurnham commented Jan 7, 2019

@ddbeck OK, I think I've addressed all of your feedback.

Regarding safari_ios: Looking at iOS devices in BrowserStack, I can confirm that onunhandledrejection is supported in iOS 12.0.0. Support was likely introduced in an earlier iOS 11.x release (Caniuse says 11.4), but I can't pin that version down. Would it be better to set "version_added": "12.0.0" instead of "version_added": null?

@jpmedley
Copy link
Contributor

jpmedley commented Jan 7, 2019

@TrevorBurnham, welcome to BCD and thank you for doing this.

FYI, caniuse and Chrome Status are fine as sources if you don't have anything better. Be aware that they are second-hand sources.

Here's a search result on the commit hash from a Chromium system called Omaha Proxy. The link on that result takes you to the actual commit where the feature landed. I got the commit hash from the IDL's file history

@TrevorBurnham
Copy link
Contributor Author

FYI, caniuse and Chrome Status are fine as sources if you don't have anything better. Be aware that they are second-hand sources.

OK, them I'm going to go with 11.4 as the safari_ios version, per caniuse. I've added that version to safari_ios.json to satisfy the linter: 0f84926

@ddbeck ddbeck dismissed their stale review January 8, 2019 14:50

Feedback fixed; new review needed for browser changes

@ddbeck
Copy link
Collaborator

ddbeck commented Jan 8, 2019

@Elchi3 would you mind reviewing the browser change here? I don't feel confident about reviewing safari_ios.json additions myself

@ddbeck ddbeck added the data:browsers Data about browsers (versions, release dates, etc). This data is used for validation. label Jan 8, 2019
@Elchi3
Copy link
Member

Elchi3 commented Jan 8, 2019

hm, I have no idea whether or not these are versions that we would care about, or just minor updates without feature changes. Does anyone have Safari release notes handy? We need to find out which of the 11.x releases are major releases with web platform feature changes.

Caniuse seems to have some grouping:
"11.0-11.2"
"11.3-11.4"
For us this would mean we would add only add 11.0 and 11.3, but we've already added 11.1...

@ddbeck
Copy link
Collaborator

ddbeck commented Jan 28, 2019

This is also related to #2006.

@queengooborg queengooborg added the not ready This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. label Apr 13, 2019
@TrevorBurnham TrevorBurnham force-pushed the update-unhandled-rejection-support branch from 0f84926 to 354f11f Compare May 1, 2019 15:20
@TrevorBurnham
Copy link
Contributor Author

@vinyldarkscratch Per your comment, I've changed the iOS version specified here from "11.4" to to "11.3" and rebased against master.

Let me know if any other changes are needed here.

@TrevorBurnham TrevorBurnham force-pushed the update-unhandled-rejection-support branch from 354f11f to 8ae6aeb Compare May 1, 2019 15:25
@queengooborg queengooborg removed data:browsers Data about browsers (versions, release dates, etc). This data is used for validation. not ready This is not yet ready to be merged. It's pending a decision, other PR, or a prerequisite action. rebase-needed 🚧 labels May 1, 2019
@queengooborg
Copy link
Contributor

Awesome, thanks for the update, and thank you for the updates! Since this no longer makes any changes to the browsers, I think that this is ready.

@ddbeck Since you know much more about this PR than I do (and the PR looks great to me), want to give it one final review?

@queengooborg queengooborg requested a review from ddbeck May 1, 2019 18:00
Copy link
Contributor

@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.

👍

Copy link
Collaborator

@ddbeck ddbeck left a comment

Choose a reason for hiding this comment

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

Thank you so much, @TrevorBurnham! This was one of the longer-lived PRs in BCD, so I really appreciate your patience on this. Let's merge this! 🎉

@ddbeck ddbeck merged commit c1f8752 into mdn:master May 1, 2019
@TrevorBurnham TrevorBurnham deleted the update-unhandled-rejection-support branch May 1, 2019 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:api Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants