1902377 - Rename "isRoot" argument of "getBrowsingContextInfo()" to "includeParentId"

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, task, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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.

Comment 1

[Liam DeBeasi]

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?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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!

Comment 3

[Liam DeBeasi]

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?

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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!

Comment 5

[Liam DeBeasi]

Attachment: Bug 1902377 - Rename "isRoot" argument to "includeParentId" r=whimboo

Comment 6

[Liam DeBeasi]

(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.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a8e9c40e536
Rename "isRoot" argument to "includeParentId" r=whimboo,webdriver-reviewers

Comment 9

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/3a8e9c40e536

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(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!

Comment 11

[Liam DeBeasi]

(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.