Skip to content
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

Add support for "userContexts" argument to "browsingContext.setViewport" command #851

Open
lutien opened this issue Jan 10, 2025 · 4 comments
Labels
enhancement New feature or request module-browsingContext Browsing Context module needs-discussion Issues to be discussed by the working group

Comments

@lutien
Copy link
Contributor

lutien commented Jan 10, 2025

Similar to other commands (e.g. session.subscribe) we should add a possibility to set viewport settings to all existing and future browsing contexts belonging to certain userContexts.

@lutien lutien added enhancement New feature or request module-browsingContext Browsing Context module needs-discussion Issues to be discussed by the working group labels Jan 10, 2025
@jgraham
Copy link
Member

jgraham commented Jan 10, 2025

In retrospect it's a bit unfortunate that we put this command in browsingContext. Of course it's only an aesthetic thing and we can easily just add a userContext field, but I wonder if we should have an emulation module with a new version of this command along with the commands for timezone overrides and so on, and deprecate the browsingContext version.

@OrKoN
Copy link
Contributor

OrKoN commented Jan 10, 2025

I think it might be better to add viewport as an attribute (defaultViewport) for the creation of user contexts unless there is a use case for changing viewport for multiple contexts at once (which I do no see).

@jgraham
Copy link
Member

jgraham commented Jan 10, 2025

I think we need a coherent plan for how the group of related things (e.g. viewports, geolocation emulation, timezone overrides, preload scripts, event subscriptions) will work.

My concern with having creating a user context allow specifying the viewport configuration is that if we apply the same pattern elsewhere we'll end up with duplication for all the things that we want to configure; one command for targeting a specific context or set of contexts, and one property on user context creation. Or alternatively we'll have a mixed model where some things like viewport overrides are specified when creating a user context, but other things that people want to apply to an entire user context like preload scripts, or event subscriptions, are separate commands.

The pattern we've adopted so far is to avoid single "configure all the things" commands (except capabilities, which I think have turned out to be a generally problematic model beyond the use case of picking a browser instance), and had single commands per configuration option. So my prior is that's the model we should keep in the future.

@OrKoN
Copy link
Contributor

OrKoN commented Jan 10, 2025

I generally I agree but I also think changing viewport in multiple existing browsing contexts might get somewhat complicated w.r.t. the background tab throttling (I could be overthinking).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request module-browsingContext Browsing Context module needs-discussion Issues to be discussed by the working group
Projects
None yet
Development

No branches or pull requests

3 participants