Rename "isRoot" argument of "getBrowsingContextInfo()" to "includeParentId"
Categories
(Remote Protocol :: WebDriver BiDi, task, P3)
Tracking
(firefox132 fixed)
Tracking | Status | |
---|---|---|
firefox132 | --- | fixed |
People
(Reporter: whimboo, Assigned: ldebeasi, Mentored)
References
()
Details
(Whiteboard: [lang=js][webdriver:m12][webdriver:external])
Attachments
(1 file)
On https://github.com/w3c/webdriver-bidi/pull/726/ a change for WebDriver BiDi landed which renamed the isRoot
argument of the getBrowsingContextInfo()
method so it's better to understand. We should follow the name and use includeParentId
.
Bug 1902206 is adapting the tests but not a hard blocker to get this rename done.
Reporter | ||
Updated•4 months ago
|
Assignee | ||
Comment 1•29 days ago
|
||
Hey there,
I am interested in submitting a patch for this. Sounds like the work required here is to rename isRoot
to includeParentId
and then update any references to the variable?
Reporter | ||
Comment 2•29 days ago
|
||
Hi Liam! It's great to see your interest to help with this bug. Your analysis is right. All these need to be updated. Once done you should as well make sure to run the tests at testing/web-platform/tests/webdriver/bidi/
to ensure that they all work. Some will most likely start passing again so that an update of the appropriate manifest file (same filename with suffix .ini
under /testing/web-platform/meta) is needed.
If you need further help please let me know or join us on Matrix in the webdriver channel. Thanks!
Assignee | ||
Comment 3•22 days ago
|
||
Thanks! I made my changes and ran the tests in testing/web-platform/tests/webdriver/bidi/
, and everything is passing. You mentioned that some tests may start passing where they previously were not, but I did not see any tests like that when I ran them. Do you have an idea of which tests need to be changed with their .ini
files?
Reporter | ||
Comment 4•22 days ago
|
||
That's great! So it should be all fine then. There are indeed no more tests that will pass now given that the actual change doesn't affect the BiDi API but just changes an internal argument name. As such when you can upload the patch to Phabricator I can push a try build to verify in CI that's all fine. Thanks for working on this bug Liam!
Assignee | ||
Comment 5•18 days ago
|
||
Updated•18 days ago
|
Assignee | ||
Comment 6•18 days ago
|
||
(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #4)
That's great! So it should be all fine then. There are indeed no more tests that will pass now given that the actual change doesn't affect the BiDi API but just changes an internal argument name. As such when you can upload the patch to Phabricator I can push a try build to verify in CI that's all fine. Thanks for working on this bug Liam!
Thanks for the clarification! I have uploaded my work to Phabricator.
Reporter | ||
Comment 7•16 days ago
|
||
Liam, thank a lot for your patch. In case you are looking for something else there is bug 1916498 for BiDi. This would be a great enhancement for our code base. So if you like to work on it please let me/us know.
Comment 9•15 days ago
|
||
bugherder |
Reporter | ||
Updated•11 days ago
|
Reporter | ||
Comment 10•10 days ago
|
||
(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #7)
Liam, thank a lot for your patch. In case you are looking for something else there is bug 1916498 for BiDi. This would be a great enhancement for our code base. So if you like to work on it please let me/us know.
Hi Liam, sorry for pinging again but maybe you missed that comment from last week. In case you have interest please let us know. Thanks!
Assignee | ||
Comment 11•9 days ago
|
||
(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #10)
(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #7)
Liam, thank a lot for your patch. In case you are looking for something else there is bug 1916498 for BiDi. This would be a great enhancement for our code base. So if you like to work on it please let me/us know.
Hi Liam, sorry for pinging again but maybe you missed that comment from last week. In case you have interest please let us know. Thanks!
Hey there! Sorry for not responding sooner. I would be more than happy to help out on https://bugzilla.mozilla.org/show_bug.cgi?id=1916498. I'll take a look at the ticket and post any questions I have there.
Description
•