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 compat data for ServiceWorkerMessageEvent #1013

Merged
merged 4 commits into from
Feb 16, 2018
Merged

Add compat data for ServiceWorkerMessageEvent #1013

merged 4 commits into from
Feb 16, 2018

Conversation

bunnybooboo
Copy link
Contributor

@Elchi3 Elchi3 added the data:api 🐇 Compat data for Web APIs. https://developer.mozilla.org/docs/Web/API label Feb 12, 2018
teoli2003
teoli2003 previously requested changes Feb 12, 2018
View reviewed changes
Copy link
Contributor

@teoli2003 teoli2003 left a comment

Choose a reason for hiding this comment

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

Really good, a few things to change, but we are close to land this.

Good and correct use of "version_removed".

api/ServiceWorkerMessageEvent.json Outdated
},
"edge": {
"version_added": "17",
"notes": "Service workers is available in Microsoft Edge starting EdgeHTML 16 behind a flag."
Copy link
Contributor

Choose a reason for hiding this comment

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

"flags"

Copy link
Contributor Author

@bunnybooboo bunnybooboo Feb 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm. As the data in the table & notes only refers to the above, how do I shape this as a flag? Previously (Fx example earlier today) I've had:

"type": "preference",  // known
"name": "dom.payments.request.enabled",  // unknown in this Edge example
"value_to_set": "true" // unknown in this Edge example

Does it get added as a note? Looking in Edge release notes, the flag is not clear to a newbie like me. 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh I think you answered this elsewhere. Wrapping my head around it. No need to respond to me now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is it (indentation not correct):

"edge": [ {
"version_added": "17"
},
{
"version_added: "16",
"flags": [ {
"type": "preference",
"name": "Enable service workers"
} ]
}
]

api/ServiceWorkerMessageEvent.json Outdated
"firefox": {
"version_added": "44",
"version_removed": "55",
"notes": "Service workers (and Push) have been disabled in the Firefox 45 and 52 Extended Support Releases (ESR)."
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add a 2nd note saying that: "In Firefox 55 and later, the standard <code>MessageEvent</code> interface must be used instead." (only on the interface)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actioning the rest of this now

api/ServiceWorkerMessageEvent.json Outdated
},
"edge": {
"version_added": "17",
"notes": "Service workers is available in Microsoft Edge starting EdgeHTML 16 behind a flag."
Copy link
Contributor

Choose a reason for hiding this comment

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

"flags"

api/ServiceWorkerMessageEvent.json Outdated
},
"edge": {
"version_added": "17",
"notes": "Service workers is available in Microsoft Edge starting EdgeHTML 16 behind a flag."
Copy link
Contributor

Choose a reason for hiding this comment

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

"flags"

api/ServiceWorkerMessageEvent.json Outdated
},
"edge": {
"version_added": "17",
"notes": "Service workers is available in Microsoft Edge starting EdgeHTML 16 behind a flag."
Copy link
Contributor

Choose a reason for hiding this comment

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

"flags"

api/ServiceWorkerMessageEvent.json Outdated
},
"edge": {
"version_added": "17",
"notes": "Service workers is available in Microsoft Edge starting EdgeHTML 16 behind a flag."
Copy link
Contributor

Choose a reason for hiding this comment

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

"flags"

api/ServiceWorkerMessageEvent.json Outdated
},
"edge": {
"version_added": "17",
"notes": "Service workers is available in Microsoft Edge starting EdgeHTML 16 behind a flag."
Copy link
Contributor

Choose a reason for hiding this comment

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

"flags"

api/ServiceWorkerMessageEvent.json Outdated
},
"firefox": {
"version_added": "44",
"notes": "Service workers (and Push) have been disabled in the Firefox 45 and 52 Extended Support Releases (ESR)."
Copy link
Contributor

Choose a reason for hiding this comment

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

"version_removed" is missing here.

api/ServiceWorkerMessageEvent.json Outdated
},
"edge": {
"version_added": "17",
"notes": "Service workers is available in Microsoft Edge starting EdgeHTML 16 behind a flag."
Copy link
Contributor

Choose a reason for hiding this comment

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

"flags"

api/ServiceWorkerMessageEvent.json Outdated
},
"firefox": {
"version_added": "44",
"notes": "Service workers (and Push) have been disabled in the Firefox 45 and 52 Extended Support Releases (ESR)."
Copy link
Contributor

Choose a reason for hiding this comment

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

"version_removed" is missing here.

jpmedley
jpmedley suggested changes Feb 12, 2018
View reviewed changes
api/ServiceWorkerMessageEvent.json Outdated
"version_added": "24"
},
"opera_android": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be 24.

api/ServiceWorkerMessageEvent.json Outdated
"version_added": "24"
},
"opera_android": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

24

api/ServiceWorkerMessageEvent.json Outdated
"version_added": "24"
},
"opera_android": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

24

api/ServiceWorkerMessageEvent.json Outdated
"version_added": "24"
},
"opera_android": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

24

api/ServiceWorkerMessageEvent.json Outdated
"version_added": "24"
},
"opera_android": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

24

api/ServiceWorkerMessageEvent.json Outdated
"version_added": "24"
},
"opera_android": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

24

api/ServiceWorkerMessageEvent.json Outdated
"version_added": "24"
},
"opera_android": {
"version_added": null
Copy link
Contributor

Choose a reason for hiding this comment

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

24

@bunnybooboo bunnybooboo mentioned this pull request Feb 13, 2018
Merged
@Elchi3
Copy link
Member

Elchi3 commented Feb 16, 2018

Looks good to me. If I see it correctly, Joe's comments have not been addressed yet, but otherwise this is good to go.

@Elchi3 Elchi3 dismissed teoli2003’s stale review February 16, 2018 10:22

comments addressed

Elchi3
Elchi3 approved these changes Feb 16, 2018
View reviewed changes
Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

Well done! Thanks :)

@Elchi3 Elchi3 merged commit 00189d8 into mdn:master Feb 16, 2018
@bunnybooboo bunnybooboo deleted the ServiceWorkerMessageEvent branch February 16, 2018 10:38
dontcallmedom pushed a commit to dontcallmedom/browser-compat-data that referenced this pull request Mar 17, 2018
@bunnybooboo @dontcallmedom
Add compat data for ServiceWorkerMessageEvent (mdn#1013)
d031836
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@teoli2003 teoli2003 teoli2003 left review comments

@jpmedley jpmedley jpmedley requested changes

@Elchi3 Elchi3 Elchi3 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.
4 participants
@bunnybooboo @Elchi3 @teoli2003 @jpmedley