Skip to content

emulation.setUserAgentOverride #947

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

emulation.setUserAgentOverride #947

wants to merge 8 commits into from
+139 −19

Conversation

sadym-chromium
Copy link
Contributor

@sadym-chromium sadym-chromium commented Jul 7, 2025 •
edited by pr-preview bot
Loading

Specification for emulation.setUserAgentOverride command.

Preview | Diff

sadym-chromium
sadym-chromium commented Jul 7, 2025
View reviewed changes
@sadym-chromium sadym-chromium force-pushed the sadym/emulation.setUserAgentOverride branch from e22449c to 989153d Compare July 7, 2025 16:04
@sadym-chromium sadym-chromium mentioned this pull request Jul 7, 2025
Open
sadym-chromium
sadym-chromium commented Jul 10, 2025
View reviewed changes
@lutien lutien mentioned this pull request Jul 11, 2025
Open
@sadym-chromium sadym-chromium force-pushed the sadym/emulation.setUserAgentOverride branch from 8cb52cd to cb5185a Compare July 23, 2025 07:53
@sadym-chromium sadym-chromium force-pushed the sadym/emulation.setUserAgentOverride branch 2 times, most recently from 9d80a52 to 7c20e1a Compare August 19, 2025 10:35
This was referenced Aug 19, 2025
Draft
Draft
@sadym-chromium
Copy link
Contributor Author

@OrKoN @juliandescottes please take a look

@sadym-chromium sadym-chromium force-pushed the sadym/emulation.setUserAgentOverride branch from 7c20e1a to b024ff2 Compare August 19, 2025 12:19
@sadym-chromium sadym-chromium mentioned this pull request Aug 22, 2025
Draft
@sadym-chromium
Copy link
Contributor Author

@juliandescottes please take a look

@sadym-chromium
Copy link
Contributor Author

@juliandescottes @whimboo PTAL

juliandescottes
juliandescottes requested changes Aug 28, 2025
View reviewed changes
Copy link
Contributor

@juliandescottes juliandescottes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sadym-chromium ! Overall that looks fine to me. Just some nits and suggestions. Before approving I just want to check the reason why we attach this to the session rather than to the remote end. Back to you

@sadym-chromium
emulation.setUserAgentOverride
3652916 
Initial draft of `emulation.setUserAgentOverride` command. Requires defining a list of related navigables for the "default User-Agent value" caller.

# Conflicts:
#	index.bs
@sadym-chromium
Try to access [=default User-Agent value=] callees
45b3081
@sadym-chromium
Allow for global setting + use struct
9cfb02e
@sadym-chromium sadym-chromium force-pushed the sadym/emulation.setUserAgentOverride branch from b024ff2 to 9cfb02e Compare August 28, 2025 14:27
sadym-chromium and others added 2 commits August 28, 2025 16:28
@sadym-chromium sadym-chromium force-pushed the sadym/emulation.setUserAgentOverride branch from b9c9a42 to 017d873 Compare August 28, 2025 14:30
@sadym-chromium
Copy link
Contributor Author

@juliandescottes PTAL I believe I addressed your comments, thanks!

juliandescottes
juliandescottes approved these changes Aug 28, 2025
View reviewed changes
Copy link
Contributor

@juliandescottes juliandescottes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I didn't notice that the context/contexts error was also in the other commands. Thanks for fixing them as well. Looks good to me now!

whimboo
whimboo reviewed Aug 28, 2025
View reviewed changes
index.bs

1. If |command parameters| [=map/contains=] "<code>userContexts</code>"
and |command parameters| [=map/contains=] "<code>contexts</code>",
return [=error=] with [=error code=] [=invalid argument=].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to fail if whether userContexts nor contexts are given.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(not in this case, if none are given we set it globally)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait. So emulating the user agent will be the first emulation command that will allow us to set some overrides globally? Is that something that we discussed / agreed on - maybe I missed it because I have to leave early from the W3C meetings?

index.bs

1. Let |emulated user agent| be |command parameters|["<code>userAgent</code>"].

1. If |command parameters| [=map/contains=] "<code>contexts</code>":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind using the same phrasing as for all the other emulation commands?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whimboo whimboo whimboo left review comments

@juliandescottes juliandescottes juliandescottes approved these changes

@OrKoN OrKoN OrKoN approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sadym-chromium @whimboo @OrKoN @juliandescottes