Mink icon indicating copy to clipboard operation
Mink copied to clipboard

Add opportunity to provide all values for a cookie in setCookie method

Open stukalin opened this issue 8 years ago • 6 comments

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.

stukalin avatar Oct 27 '16 13:10 stukalin

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

aik099 avatar Oct 27 '16 13:10 aik099

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?

lauft avatar Nov 08 '16 23:11 lauft

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.

aik099 avatar Nov 09 '16 07:11 aik099

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.

stof avatar Nov 09 '16 08:11 stof

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.

jcalderonzumba avatar Dec 05 '16 08:12 jcalderonzumba

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!

mikemix avatar Feb 08 '19 14:02 mikemix