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 more chrome flags for testing. #1416
Conversation
@paulirish I'm still using the chrome launcher in my application and I already add |
'--disable-translate', | ||
'--disable-default-apps', | ||
'--no-first-run', | ||
'--disable-extensions', // disable all chrome extensions entirely |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure how strongly I feel about it, but comments above each line might be easier to read here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
'--disable-default-apps', | ||
'--no-first-run', | ||
'--disable-extensions', // disable all chrome extensions entirely | ||
// '--ignore-certificate-errors', // Ignore certificate errors, like self-signed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm personally fine with leaving this in while commented out, but I feel strongly we shouldn't have it on by default. It's just not worth being responsible for turning this on for all users for the few who need it. In a pure testing environment this should be fine, but there are many using LH on arbitrary sites on personal machines, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wardpeet mentioned maybe exposing this as a CLI flag. Talking about it offline with @brendankenny, we could do a --chrome-flags="--ignore-certificate-errors"
kinda thing. That's not bad at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should make it a bit more general. Ignore ssl certs you probably only want it in dev mode. Maybe there are other options we want? We could expose it as --dev
like we do with --mobile
it could underneath use --chrome-flags="--ignore-certificate-errors"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed ignore-certificate-errors from this PR.
filed #1443 for --chrome-flags
extension updating, safe browsing service, upgrade detector, translate, UMA */ | ||
'--disable-sync', // disable syncing to a Google account | ||
'--metrics-recording-only', // disables reporting to UMA, but allows for collection | ||
'--safebrowsing-disable-auto-update', // New safebrowsing lists will not be fetched |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
supposedly this is redundant due to --disable-background-networking
(crbug.com/354743), but I guess it doesn't hurt, either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree
PTAL |
I was inspired by https://cs.chromium.org/chromium/src/chrome/test/chromedriver/chrome_launcher.cc?sq=package:chromium&dr=C&l=64 which listed a bunch of flags we aren't using yet.
I've added the ones that make sense, along with comments to explain a few.
cc @Janpot.. what do you think about flipping on
ignore-certificate-errors
for everyone here? I'm okay with it if you are. :)