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 opportunity to provide all values for a cookie in setCookie method #724

Open
stukalin opened this issue Oct 27, 2016 · 6 comments
Open

Comments

@stukalin
Copy link

stukalin commented Oct 27, 2016

Currently setCookie accepts only name and value. I suggest extending this so that the method accepts all other possible parameters: domain and expiry (not sure about secure though) as described here.

The change implies adding parameters to the inteface DriverInterface and implementation in drivers.

@aik099
Copy link
Member

aik099 commented Oct 27, 2016

Not sure if setting other parameters on a cookie is supported by all drivers.

@lauft
Copy link

lauft commented Nov 8, 2016

Looking at the function definition of DriverInterface::setCookie in DriverInterface.php:203 the phpdoc states that the function throws

UnsupportedDriverActionException When operation not supported by the driver
DriverException When the operation cannot be done

So if setting name or value is not supported by a driver there is an exception for that. Couldn't that be expanded to domain and expiry?

@aik099
Copy link
Member

aik099 commented Nov 9, 2016

Not sure, because MinkExtension for example is making setCookie calls in driver agnostic ways and can always pass all parameters. This way it might break all existing calls.

Maybe setCookie signature could be changed to make default values for all new parameters as null and then MinkExtension pass null (or nothing). Then we'll throw DriverException based on fact, that specific new parameter has non-null value.

Maybe that even will work as expected. However all cookie-related driver test suite tests also needs to be updated to support it.

@stof
Copy link
Member

stof commented Nov 9, 2016

The question is which driver support setting these. If only 1 driver can implement the abstraction fully, it is a bad abstraction. This is why we ask you about driver support for this new feature.

@jcalderonzumba
Copy link
Contributor

My two cents here, as far as i know, Selenium, PhantomJS and ZombieJS support cookies with more than just name, value. I kind of assume that the driver BrowserKit/Goutte will also be able to support this because in the end is "just" a header that need to be sent to the server.

@mikemix
Copy link

mikemix commented Feb 8, 2019

Yeah this sucks. Cannot set custom domain for the cookie! I have to override the setCookie method of a driver to accomplish such trivial thing, override the factory and finally override the extension. Really tedious just to add one line to the cookie config array!

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

No branches or pull requests

6 participants