-
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
Edge webextension api version updates #1486
Conversation
webextensions/api/storage.json
Outdated
"version_added": "14", | ||
"notes": [ | ||
+ "storage.local data is persisted in a different format than Chrome, causing a different value to be returned when calling storage.local.getBytesInUse. Ex: storage.local.set({ "k": { "s": "âas" } } returns 13 in Chrome and 50 in Microsoft Edge.", | ||
+ ] |
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 +
es at the beginning of the lines are causing JSON parsing errors.
webextensions/api/tabs.json
Outdated
"version_added": "14", | ||
"notes": [ | ||
+ "After uninstall/reinstall, the URL is not received until Microsoft Edge is restarted." | ||
+ ] |
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.
webextensions/api/tabs.json
Outdated
"version_added": "14", | ||
"notes": [ | ||
+ "`pinned` and `muted` are not yet supported. `highlighted` and `selected` are not supported." | ||
+ ] |
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.
webextensions/api/webRequest.json
Outdated
+ "Network requests from extensions, such as options, background or popup pages, are not supported.", | ||
"Network requests from <object> and <embed> elements are not supported.", | ||
"Headers cannot be modified for cached requests." | ||
+ ] |
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.
webextensions/api/storage.json
Outdated
+ "storage.local data is persisted in a different format than Chrome, causing a different value to be returned when calling storage.local.getBytesInUse. Ex: storage.local.set({ "k": { "s": "âas" } } returns 13 in Chrome and 50 in Microsoft Edge.", | ||
+ ] | ||
"storage.local data is persisted in a different format than Chrome, causing a different value to be returned when calling storage.local.getBytesInUse." | ||
] |
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.
These lines have wrong indentation.
webextensions/api/webRequest.json
Outdated
+ "`webRequest` not supported for synchronous XmlHttpRequests.", | ||
+ "Network requests from extensions, such as options, background or popup pages, are not supported.", | ||
"`webRequest` not supported for synchronous XmlHttpRequests.", | ||
"Network requests from extensions, such as options, background or popup pages, are not supported.", | ||
"Network requests from <object> and <embed> elements are not supported.", | ||
"Headers cannot be modified for cached requests." |
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.
ditto
webextensions/api/storage.json
Outdated
@@ -77,7 +77,7 @@ | |||
"edge": { | |||
"version_added": "14", | |||
"notes": [ | |||
"storage.local data is persisted in a different format than Chrome, causing a different value to be returned when calling storage.local.getBytesInUse." | |||
"<code>storage.local</code> data is persisted in a different format than Chrome, causing a different value to be returned when calling <code>storage.local.getBytesInUse</code>." | |||
] |
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.
These lines still have the wrong indentation.
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.
Thanks, this is an awesome PR! I had a few comments.
I don't know in general how to get detailed version info for Edge, or any detailed Edge compat info really beyond https://docs.microsoft.com/en-us/microsoft-edge/extensions/api-support/supported-apis.
14 seems like a reasonable baseline, so in general "true"->"14" seems like a good change. There are a few instances where you have changed "15"->"14" - I assume these are correct too, but please make sure :-)
"version_added": true | ||
"version_added": true, | ||
"notes": [ | ||
"tab object in runtime.onMessage event is not fully implemented.", |
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 doesn't seem like a very helpful note, is it possible to be more specific about what isn't supported?
@@ -228,7 +228,7 @@ | |||
"version_added": true | |||
}, | |||
"edge": { | |||
"version_added": false | |||
"version_added": "14" |
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.
https://docs.microsoft.com/en-us/microsoft-edge/extensions/api-support/supported-apis#webnavigation, in the summary at the top, says transition types are not supported, but the entry for TransitionType does not. I don't know which is correct here.
"version_added": true | ||
"version_added": "14", | ||
"notes": [ | ||
"webRequest not supported for synchronous XmlHttpRequests." |
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.
"webRequest not supported for synchronous XmlHttpRequests." -> "webRequest events are not fired for synchronous XMLHttpRequests." or something like that?
@@ -1541,9 +1544,10 @@ | |||
}, | |||
"edge": { | |||
"notes": [ | |||
"Asynchronous event listeners are not supported." | |||
"Asynchronous event listeners are not supported.", | |||
"`requestBody` is not supported." |
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.
Should use <code>requestBody</code>
, or just plaintext. Markdown is not supported in notes (yes I know lots of existing notes use it :-/
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 could probably be merged with my effort in #1686.
This needs a rebase now that #1686 is merged. |
No activity here, so I'm closing this now. Please re-open if you want to come back to it. |
No description provided.