gigebyte / cookies

Automatically exported from code.google.com/p/cookies
0 stars 0 forks source link

Error in set method if the value parameter is numeric #17

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps reproduce the problem?
1.Try to set a cookie with a numeric value (not as string).

What is the expected result of the above steps?  What do you see instead?
Expected: The cookie been set.
Obtained: Error Thrown

What version of the following are you using?
  jQuery: 1.3.2
  cookies: 2.2.0

What browser/version are you using?
Internet Explorer 7

What OS/version are you using?
Windows XP

Do you have any additional information to provide?
In the setter, is necessary to check if the data type of the cookie value is a 
number, not only string.

Fix:
In line 221 of jquery.cookies.2.2.0.js, replace:
else if( typeof value !== 'string' )

With:
else if( typeof value !== 'string' && typeof value !== 'number' )

Original issue reported on code.google.com by javierza...@gmail.com on 12 Jan 2010 at 9:41

GoogleCodeExporter commented 9 years ago
Hmmm, figures it's IE.  All my unit testing is currently for Firefox--I really 
need to port it to something else.

Anyway, I will definitely take a look at this, but the problem with your 
suggested fix is that a value passed in 
as an integer would return as a numeric string.

I will look further into JSON encoding standards and which browser is better 
obeying, as well as how to work 
around this.

Thanks,
Jim

Original comment by auldrid...@gmail.com on 12 Jan 2010 at 5:12

GoogleCodeExporter commented 9 years ago
javierzapata82,

I am unable to reproduce the problem you're having.  Using IE7 in Windows XP, I 
set a cookie with a numeric 
(non-string) value and no error is thrown.  Further, when I retrieve the cookie 
I get the correct value of the 
correct type.

I have created two cases based on your description: 
http://jaaulde.com/test_bed/cookietest/issue17.html

Does that page run correctly for you?  Does it look like what you were trying 
to do?

Thanks for any further info you can provide.
Jim

Original comment by auldrid...@gmail.com on 12 Jan 2010 at 8:02

GoogleCodeExporter commented 9 years ago
Ok, I think I found the problem.

It works fine as long as you include the JSON parser. If not, that last "if" 
will evaluate "false", thus throwing the 
Error.

It seems that only affects IE7 and lower (tested in IE7-IE8-FF3.5 Win and 
Safari-FF3.5 Mac).

Regards,
Javier.

Original comment by javierza...@gmail.com on 13 Jan 2010 at 1:20

GoogleCodeExporter commented 9 years ago
Hello Javier,

Yes, you must provide the JSON API to browsers which do not have it natively.  
That is why I link my JSON wiki 
page ( http://code.google.com/p/cookies/wiki/JSON ) throughout the docs when 
JSON is mentioned.  I just 
went through that page editing in an attempt to be more clear, so hopefully 
this won't be a source of 
confusion in the future.

Thanks for getting back to me.
JIm

Original comment by auldrid...@gmail.com on 13 Jan 2010 at 1:46

GoogleCodeExporter commented 9 years ago

Original comment by auldrid...@gmail.com on 13 Jan 2010 at 1:47

GoogleCodeExporter commented 9 years ago
Sorry, my mistake for not reading the changelog :)

Thanks!

Original comment by javierza...@gmail.com on 13 Jan 2010 at 3:12

GoogleCodeExporter commented 9 years ago
Jim,
Thanks for your work on this plugin, saved me time! Although you're right that 
adding 
the JSON API fixes the problem, I'd recommend you make the "Number" case an 
exception 
as said in the original comment. It's so common to allow javascript to cast 
from 
string to number and from number back to string that it feels very awkward 
having to 
do it manually when not using the JSON API. For example, here's a simple 
client-side 
counter that I wrote with the cookies plugin:
var myNum = Number($.cookies.get('myNum')).valueOf();
if (!myNum) {
  myNum = 1;
}
else {
  myNum++;
}
$.cookies.set('myNum', String(myNum).toString(), {expiresAt: new Date( 2015, 1, 
1 
)});

Just a suggestion, but it feels like a case where we could let Javascript do 
the work 
for us.

Original comment by s...@andrewrlevine.com on 26 Jan 2010 at 7:58