Add WebDriver BiDi data for command/event parameters#26993
Conversation
|
Tip: Review these changes grouped by change (recommended for most PRs), or grouped by feature (for large PRs). |
0374a19 to
28b2fd3
Compare
e5400cf to
a75400a
Compare
071aab0 to
ea6f748
Compare
|
This pull request has merge conflicts that must be resolved before it can be merged. |
5b46134 to
e749028
Compare
caugner
left a comment
There was a problem hiding this comment.
Reviewed the changes, and noted especially the following cases:
- (More important) Noticable version changes or status changes.
- (Less urgent) In some cases, parameters are partially supported, but there is no note explaining the limitation (should answer: what works, what doesn't work). (This can be recorded in the spreadsheet using a "Note" in the version cell.)
@whimboo @sadym-chromium Let me know if you have any questions. 🙏
| "version_added": "101", | ||
| "partial_implementation": true, | ||
| "notes": "Basic support (see [bug 1730641](https://bugzil.la/1730641))" | ||
| "version_added": "101" |
There was a problem hiding this comment.
@whimboo Before, Firefox data for browsingContext.navigate() indicated partial support from 101, now it states full support (including all parameters) in that version. Correct?
There was a problem hiding this comment.
As the meta bug shows we have had a lot of follow-up fixes for this command and we are not done yet with all of them. I don't think that we should list each and every improvement in the note to this command. The initial landing contained support for all the parameters. Do you think otherwise?
There was a problem hiding this comment.
I think it might be helpful for developers to mention the most relevant limitations (which doesn't imply marking the feature as partial).
For example:
Does not handle navigations that start a download (bug 1763126), navigations to javascript URLs (bug 1763131), or navigations where the initial request never comes back or times out (bug 1763135).
| "version_added": false | ||
| "version_added": "135", | ||
| "partial_implementation": true, | ||
| "notes": "Basic support" |
There was a problem hiding this comment.
@sadym-chromium Would be good to add details about Chrome's partial implementation of the webExtension.install() command, ideally linking to a bug.
| "chrome": { | ||
| "version_added": "135", | ||
| "partial_implementation": true, | ||
| "notes": "Basic support" |
There was a problem hiding this comment.
@sadym-chromium Would be good to add details about Chrome's partial implementation of the extensionData parameter of the webExtension.install command, ideally linking to a bug.
There was a problem hiding this comment.
We don't plan to implement other types of extensionData, so no tracking bug there.
| "chrome": { | ||
| "version_added": "135", | ||
| "partial_implementation": true, | ||
| "notes": "Basic support" |
There was a problem hiding this comment.
@sadym-chromium Would be good to add details about Chrome's partial implementation of the extension parameter of the webExtension.uninstall command, ideally linking to a bug.
There was a problem hiding this comment.
I marked it as partial, as unlike our e2e tests, the WPT tests are not passing. It needs som investigation for extended status.
| "firefox": { | ||
| "version_added": "122", | ||
| "partial_implementation": true, | ||
| "notes": "Basic support" |
There was a problem hiding this comment.
@whimboo Would be good to add details about Firefox's partial implementation of the response parameter of the network.authRequired event, ideally linking to a bug.
67f4e65 to
fd0ca91
Compare
1cb0e38 to
b6d2489
Compare
|
@caugner I assume this PR still needs another update to fetch the latest changes from the spreadsheet. |
d29c8b9 to
dd0bb70
Compare
There was a problem hiding this comment.
Thank you @whimboo @sadym-chromium @juliandescottes for your swift input! 🙌
The new parameter support data is looking good now, so let's land this as is, and follow up on the remaining open review threads later. 🎉
(This PR was generated by the
update-webdriver-bidi-dataworkflow.)