Change default value of --webbrowser-headless to False #6116
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add `headless=False` keyword to `Webbrowser.launch()` - Remove `headless` keyword from `WebbrowserChromium` constructor - Include headless mode in log output when launching - Update tests
gravyboat
approved these changes
Aug 6, 2024
There was a problem hiding this comment.
Opinions on the log message and updated argument help text? Should this be made more clear?
I think the log message and updated argument help text is fine. Most people aren't going to use this any way so if someone is they should know what they're doing. As you said it's logged every time at info level so anything more would be pretty spammy.
If it does become a problem or people are confused it can be updated at that time. Until then this should be plenty.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #6114 (comment)
Since headless mode can potentially be detected, we shouldn't set it as the default mode. Plugins should always work reliably in their default configuration without users having to change Streamlink's default options should there be a change of headless mode detection by the website a plugin covers with Streamlink's webbrowser API. It doesn't matter that a short web browser window might appear on the user's screen (in the default configuration).
So, this changes the default value of
--webbrowser-headlessfromTruetoFalseand now also logs which mode Chromium is being launched in. I don't consider this a "breaking change" (wouldn't be a real one anyway as it's only a change in behavior), as the entire API is marked as unstable, and we've also only been shipping it in the Twitch plugin which so far (still) overrides the value and sets it toFalseanyway. Recently we've updated the vtvgo plugin with a web browser requirement, but this hasn't made it into a stable release yet.I'm going to remove the override from the Twitch plugin in a follow-up PR, which means that with the new defaults, nothing will change here and users will be able to set headless mode via
--webbrowser-headless=True. Users who insisted on having a client-integrity token included in Twitch API requests so far needed to have a patched plugin version anyway, because Twitch removed the requirement after a couple of days, so the webbrowser API was never actually used by default, but with the recent addition of--twitch-force-client-integrity, users will now be able to get CI tokens with and without headless mode, or not (as long as Twitch keeps the requirement disabled).This change of default value also means that the CLI arg needs to be set explicitly in environments without a desktop (actual headless requirement).
Opinions on the log message and updated argument help text? Should this be made more clear? I don't really want this to be verbose, because it's logged on the info level every time the process is launched.