Add Cookies.removeCookie method that takes a path

112 views
Skip to first unread message

j...@google.com

unread,
May 12, 2009, 4:05:50 PM5/12/09
to j...@google.com, rj...@google.com, google-web-tool...@googlegroups.com
Reviewers: jgw, rjrjr,

Description:
This patch addresses issue 1574 (and apparently 1735 as well) by adding
a way to specify a path when removing a cookie (needed if the cookie was
set with a path).


http://code.google.com/p/google-web-toolkit/issues/detail?id=1574
http://code.google.com/p/google-web-toolkit/issues/detail?id=1735

Please review this at http://gwt-code-reviews.appspot.com/34805

Affected files:
user/src/com/google/gwt/user/client/Cookies.java
user/test/com/google/gwt/user/client/CookieTest.java


Index: user/test/com/google/gwt/user/client/CookieTest.java
===================================================================
--- user/test/com/google/gwt/user/client/CookieTest.java (revision 5352)
+++ user/test/com/google/gwt/user/client/CookieTest.java (working copy)
@@ -126,6 +126,44 @@
}

/**
+ * Test that removing cookies with a path works correctly.
+ *
+ * Note that we do not verify failure to remove a cookie set with a path
+ * but removed without one as browser behavior differs.
+ */
+ public void testRemoveCookiePath() {
+ // First clear all cookies
+ clearCookies();
+
+ // Set a few cookies
+ Cookies.setCookie("test1", "value1", null, null, "/", false);
+ Cookies.setCookie("test2", "value2", null, null, "/", false);
+ Cookies.setCookie("test3", "value3", null, null, "/", false);
+ Collection<String> cookies = Cookies.getCookieNames();
+ assertEquals(3, cookies.size());
+
+ // Remove a cookie
+ Cookies.removeCookie("test2", "/");
+ assertEquals("value1", Cookies.getCookie("test1"));
+ assertEquals(null, Cookies.getCookie("test2"));
+ assertEquals("value3", Cookies.getCookie("test3"));
+
+ // Remove another cookie
+ Cookies.removeCookie("test1", "/");
+ assertEquals(null, Cookies.getCookie("test1"));
+ assertEquals(null, Cookies.getCookie("test2"));
+ assertEquals("value3", Cookies.getCookie("test3"));
+
+ // Remove last cookie
+ Cookies.removeCookie("test3", "/");
+ assertEquals(null, Cookies.getCookie("test1"));
+ assertEquals(null, Cookies.getCookie("test2"));
+ assertEquals(null, Cookies.getCookie("test3"));
+ cookies = Cookies.getCookieNames();
+ assertEquals(0, cookies.size());
+ }
+
+ /**
* Clear out all existing cookies.
*/
private void clearCookies() {
Index: user/src/com/google/gwt/user/client/Cookies.java
===================================================================
--- user/src/com/google/gwt/user/client/Cookies.java (revision 5352)
+++ user/src/com/google/gwt/user/client/Cookies.java (working copy)
@@ -60,15 +60,28 @@
}

/**
- * Removes the cookie associated with the given name.
+ * Removes the cookie associated with the given name, if no path was
given
+ * when {@link #setCookie} was called.
*
* @param name the name of the cookie to be removed
*/
public static native void removeCookie(String name) /*-{
- $doc.cookie = name + "=;expires=Fri, 02-Jan-1970 00:00:00 GMT";
+ $doc.cookie = encodeURIComponent(name) + "=;expires=Fri, 02-Jan-1970
00:00:00 GMT";
}-*/;

/**
+ * Removes the cookie associated with the given name.
+ *
+ * @param name the name of the cookie to be removed
+ * @param path the path to be associated with this cookie (which should
match
+ * the path given in {@link #setCookie})
+ */
+ public static native void removeCookie(String name, String path) /*-{
+ $doc.cookie = encodeURIComponent(name) + "=;path=" + path
+ + ";expires=Fri, 02-Jan-1970 00:00:00 GMT";
+ }-*/;
+
+ /**
* Sets a cookie. The cookie will expire when the current browser
session is
* ended.
*


j...@google.com

unread,
May 13, 2009, 10:20:02 AM5/13/09
to j...@google.com, rj...@google.com, google-web-tool...@googlegroups.com
One comment on a test that could be shored up; otherwise, LGTM.


http://gwt-code-reviews.appspot.com/34805/diff/1/2
File user/test/com/google/gwt/user/client/CookieTest.java (right):

http://gwt-code-reviews.appspot.com/34805/diff/1/2#newcode163
Line 163: assertEquals(0, cookies.size());
Mind adding a test that would fail if we hadn't added
'encodeURIComponent()' to removeCookie()?

http://gwt-code-reviews.appspot.com/34805

Reply all
Reply to author
Forward
0 new messages