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

Make security.csp.enabled mutable (but still default it to false) #3428

Merged
merged 1 commit into from
Feb 23, 2017

Conversation

kateposener
Copy link
Contributor

With the advent of marionette/geckodriver we want to be able to run our selenium test with CSP enabled. At the moment, we have to studiously override the 'onlyOverrideThisIfYouKnowWhatYouAreDoing' function in the java firefox driver in order to do so; this is a claim that makes us nervous.

We understand the motivation for this previously being frozen; we are hoping that we can leave the default to false, but unfreeze the property. You may disagree of course - we'd love to know why :-)

@shs96c
Copy link
Member

shs96c commented Feb 14, 2017

Two questions:

  1. Does the geckodriver still work when this setting is set to true?

  2. Does executeScript still work with this setting set to true?

If either of those doesn't work, making this value mutable is a recipe for chaos.

@kateposener
Copy link
Contributor Author

Thanks for having a look at this. Regarding your questions:

  1. We have run some of our acceptance tests with geckodriver and csp.enable set to true and they continue to work. The only thing currently stopping us putting a run through our CI for further feedback is that we're having problems with geckodriver and driver.quit(). This is the case with and without csp enabled though, so csp doesn't appear to be having an impact on that.

  2. I have run a test that uses ((JavascriptExecutor)driver).executeScript() and this continues to work.

Please let me know if you require anything further.

@shs96c shs96c merged commit 72fb2e5 into SeleniumHQ:master Feb 23, 2017
@shs96c
Copy link
Member

shs96c commented Feb 23, 2017

OK. Sounds like this is safe to land. Thank you for checking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants