1976416 - Add helper to assert and transform browsing and user contexts in emulation commands

Status: RESOLVED FIXED

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

Description

[Alexandra Borovova [:Sasha]]

In the emulation commands such "setGeolocationOverride" and "setLocaleOverride" we repeat the following steps:

• assert that at least one of the contexts or user contexts are provided (and not both);
• for browsing contexts, check if it exists and top level, extract to set of navigables;
• for user contexts, check if exists, get navigables and extract to set of internal user contexts.

We could create a helper (or multiple helpers) to avoid repetition.
There is a similar algorithm in "browsingContext.setViewport" command, with the only difference that there can be only one browsing context as an argument, but the rest is similar to emulation commands, so we could probably simplify it as well.

Comment 1

[sabina]

Hello, I would like to work on this issue, can I get assigned to it?

Comment 2

[Alexandra Borovova [:Sasha]]

(In reply to sabina from comment #1)

Hello, I would like to work on this issue, can I get assigned to it?

Sure, done. Let me know if you need more information :)

Comment 3

[Alexandra Borovova [:Sasha]]

Hi Sabina!
How is it going with the bug? Do you maybe need some help?
Let us know :)

Comment 4

[sabina]

Hello, sorry for disappearing, I was busy lately. I will work on this issue this week.

Comment 5

[Alexandra Borovova [:Sasha]]

Hey, Sabina!
Just checking again if it's going fine with the bug :)

Comment 6

[sabina]

There is a similar algorithm in "browsingContext.setViewport" command, with the only difference that there can be only one browsing context as an argument, but the rest is similar to emulation commands, so we could probably simplify it as well.

Should helpers created for "setGeolocationOverride" and "setLocaleOverride" cases be applicable to "browsingContext.setViewport" as well?

Comment 7

[Alexandra Borovova [:Sasha]]

(In reply to sabina from comment #6)

Should helpers created for "setGeolocationOverride" and "setLocaleOverride" cases be applicable to "browsingContext.setViewport" as well?

Ideally, yes :) But if it gets too complicated we can move this part to follow-up.

Comment 8

[Alexandra Borovova [:Sasha]]

Hey @sabina,
do you still have time to work on the bug? If not, it's no problem, just let us know so we can reset the assignee.

Comment 9

[Sameem [:sameembaba]]

hey there! Can i work on this issue?

Comment 10

[Sameem [:sameembaba]]

I’d be happy to take this issue if it’s available. I’ve been reviewing the description and understand the repeated logic in the emulation commands (setGeolocationOverride, setLocaleOverride). I’m setting up the build environment now

Comment 11

[Alexandra Borovova [:Sasha]]

Hi Sameem, sure, go ahead! I assigned the bug to you. Please let me know if you have any questions.
Just a note that since the bug was created, we added more emulation commands that share the same assertion logic, and it would be great to update them as well.

Comment 12

[Sameem [:sameembaba]]

Attachment: Bug 1976416 - Add helper to assert and transform browsing and user contexts in emulation commands. r=Sasha

Comment 13

[Sameem [:sameembaba]]

Hi Sasha,

I’ve addressed the issues mentioned earlier and updated the patch accordingly. The refactor preserves existing behavior and validation. Please let me know if anything else needs adjustment.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/14f37b0705c9
https://hg.mozilla.org/integration/autoland/rev/4528c71d847c
Add helper to assert and transform browsing and user contexts in emulation commands. r=Sasha

Comment 15

[Sameem [:sameembaba]]

Hi Sasha,

Thanks for guiding me through this first patch! I really enjoyed working on the WebDriver BiDi module and learning about how the emulation commands work.

Since I have my build environment set up and familiarized myself with the code structure, I would love to contribute more to this component.

Do you have any other "good first bugs" or backlog tasks in WebDriver BiDi (or related remote modules) that I could tackle next?

Comment 16

[amarc]

https://hg.mozilla.org/mozilla-central/rev/4528c71d847c

Comment 17

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

Sameem thanks a lot! Great to see that we are now able to get rid of this massive code duplication.