gigebyte / cookies

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

cookify/cookieFill don't properly persist radiobuttons with both an id and a name #19

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps reproduce the problem?
1. Create a radio button with both an id an a name.
2. Call cookify() on the element.
3. Change the value or refresh the page.
4. Call cookieFill() on the element.

What is the expected result of the above steps?  What do you see instead?
I expect to see the radio button set to the stored value.  Instead nothing 
happens.  Debugging through the code shows that cookify uses "name" but 
cookieFill uses "id".

See attached file for repro case using cookieBind.

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

What browser/version are you using?
IE8

What OS/version are you using?
WIndows 7

Do you have any additional information to provide?
Line 385 should be "shift()" instead of "pop()" in order to traverse the 
attribute names in the same order as cookify().

Original issue reported on code.google.com by jwas...@gmail.com on 7 Feb 2010 at 10:44

Attachments:

GoogleCodeExporter commented 9 years ago
Taking a look, thanks for reporting and for providing your test code.

Original comment by auldrid...@gmail.com on 7 Feb 2010 at 3:51

GoogleCodeExporter commented 9 years ago
Ok, so the gist of your suggestion is to give the 'name' attr a higher 
precedence than the 'id' attr?  This does 
make sense given the problem you've run into and the way radios work.  Let me 
play around and see how that 
works out.

Original comment by auldrid...@gmail.com on 7 Feb 2010 at 4:20

GoogleCodeExporter commented 9 years ago
Precedence doesn't much matter, just that both cookify and cookeiFill used the 
same 
precedence.  For radio button, I think giving "name" precedence is the right 
thing 
to do, but I haven't tried it the other way, nor looked at non-radio-button 
situations.

Original comment by jwas...@gmail.com on 7 Feb 2010 at 6:03

GoogleCodeExporter commented 9 years ago
I've begun some work on this, but it turns out to be a bit more complicated 
(doesn't it always?)

Radios are going to have to be handled specially since those of the same name 
are mutually exclusive and, 
therefore, cannot simply be iterated end to end to determine cookie value--if 
the first is checked and the last is 
not, then the lat causes the cookie to be wiped.

This also raises some questions in my mind about checkboxes.  But I'm working 
on it....

Original comment by auldrid...@gmail.com on 3 Jun 2010 at 5:42

GoogleCodeExporter commented 9 years ago
Any progress with this issue? If you need specific help let me know.

Original comment by john.pis...@gmail.com on 12 Jun 2010 at 6:42

GoogleCodeExporter commented 9 years ago
Issue 29 has been merged into this issue.

Original comment by auldrid...@gmail.com on 10 Nov 2010 at 11:43

GoogleCodeExporter commented 9 years ago
Issue 33 has been merged into this issue.

Original comment by auldrid...@gmail.com on 19 Dec 2010 at 11:06

GoogleCodeExporter commented 9 years ago
Giving this a hard look. r77 now isolates radios from other items. figuring out 
what to do with them while allowing other elements to work properly. 7 tests 
currently failing in the test suite while I ignore radios

Original comment by auldrid...@gmail.com on 27 Jan 2011 at 4:33

GoogleCodeExporter commented 9 years ago

Original comment by auldrid...@gmail.com on 27 Jan 2011 at 4:34

GoogleCodeExporter commented 9 years ago
Note to self, look back into http://api.jquery.com/Types#Map for thoughts on 
dealing with radios.

Original comment by auldrid...@gmail.com on 8 Feb 2011 at 2:11

GoogleCodeExporter commented 9 years ago
There seems to be a problem with check-boxes also.

In the HTML file I've uploaded are two radio buttons and two check boxes with 
ids and names as created by the asp.net CheckboxList and RadioButtonList 
controls.

The Checkbox cookies are saved but they are not returned to their state when 
the page is reloaded.

Original comment by StevenWe...@gmail.com on 22 Dec 2011 at 8:51

Attachments:

GoogleCodeExporter commented 9 years ago
Changing line 385 to use shift() instead of pop() does indeed work, and I say 
this seems like the best change to make, and a VERY important one as well. The 
entire cookie library is potentially unusable otherwise.

Another change that fixed this was to swap lines 416 and 418. CookieFill() 
still checks for "id" first instead of "name", but this way it at least won't 
break form the while() loop until it has also checked "name" (in the case that 
there is no cookie value stored using the id). But still, it's best practice to 
check the values in the same order as they were checked when cookified to avoid 
confusion.

Both the above modifications have been working for me and make the most sense.

Original comment by mach...@gmail.com on 20 Jan 2012 at 9:37