-
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
add compat data for Client#frameType and Client#postMessage #1059
Conversation
api/Client.json
Outdated
"version_added": null | ||
}, | ||
"opera_android": { | ||
"version_added": null |
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.
Both versions of Opera should be 30.
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.
If I look up the Client#frameType page on MDN Opera states support has unknown? Not too sure what data source to trust?
Happy to make changes if needed. Just wanna double check
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.
api/Client.json
Outdated
"mdn_url": "https://developer.mozilla.org/docs/Web/API/Client/frameType", | ||
"support": { | ||
"webview_android": { | ||
"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.
This should be 43.
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.
The Android section on the frameType page specifies not supported. However Chrome for android says version 43.
Do you still want me to go ahead and make the change?
I believe that Chrome for Android has its own entry in the __compat object
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.
All comments fixed! Thanks @jpmedley for the feedback |
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.
Looks good to me.
I don't why this was added, but please remove the changes to package-lock.json
@Elchi3 done, I believe I might have run |
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.
Thank you!
@gsouquet I'm not sure. Maybe it's me who needs to update to a newer node or npm version? I will double check. In any case, should be discussed in a new issue if this is a problem. |
Original source: mdn#1059 No source was given, but it might have been this: https://storage.googleapis.com/chromium-find-releases-static/f77.html#f777755953e905594276a99cf64fc3848406d5f6 However, that commit was in time for Chrome 36 or 37, well before Service Workers were first enabled. So match the parent feature. Part of mdn#7844.
Original source: #1059 No source was given, but it might have been this: https://storage.googleapis.com/chromium-find-releases-static/f77.html#f777755953e905594276a99cf64fc3848406d5f6 However, that commit was in time for Chrome 36 or 37, well before Service Workers were first enabled. So match the parent feature. Part of #7844.
That should complete #980