Skip to content
This repository was archived by the owner on Nov 15, 2017. It is now read-only.

Really remove cookie whether it's created with an default domain or not#351

Closed
ryoqun wants to merge 1 commit intocarhartl:masterfrom
ryoqun:remove-stubborn-cookie
Closed

Really remove cookie whether it's created with an default domain or not#351
ryoqun wants to merge 1 commit intocarhartl:masterfrom
ryoqun:remove-stubborn-cookie

Conversation

@ryoqun
Copy link

@ryoqun ryoqun commented Mar 2, 2015

This fixes the following insanity:

screen shot 2015-03-02 at 7 02 21 pm

@ryoqun ryoqun force-pushed the remove-stubborn-cookie branch 2 times, most recently from 1e246c0 to 77382b3 Compare March 2, 2015 10:15
@ryoqun ryoqun force-pushed the remove-stubborn-cookie branch from 77382b3 to 8ca2faf Compare March 2, 2015 10:16
@Krinkle
Copy link
Contributor

Krinkle commented Mar 2, 2015

Shouldn't the application code be responsible for being consistent in how it creates, updates and removes cookies with the same options?

@carhartl
Copy link
Owner

carhartl commented Mar 2, 2015

I think so too @Krinkle. The default options may help with this.

@FagnerMartinsBrack
Copy link
Collaborator

The docs says // Returns true when cookie was successfully deleted, otherwise false.

That seems to be misleading, shouldn't it be // Returns true when target cookie exists, but may not be deleted if options were not passed correctly, or enforce somewhere that The behavior for passing wrong options to $.removeCookie is undefined. This is implicit, but still an abstraction leak from the browser default cookie mechanism, if we consider that this behavior should be known by the developer.

@carhartl
Copy link
Owner

The documentations says:

when deleting a cookie, you must pass the exact same path, domain and secure options that were used to set the cookie

I want to keep it that way, otherwise we'd be expected to do the same for path at some point, adding even more code...

Thanks for the PR, but closing.

@carhartl carhartl closed this Mar 19, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants