166560 - form checkbox are incorrrect on reloading

Status: NEW

(Core :: Layout: Form Controls, defect, P5)

Description

[Ian]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.1) Gecko/20020826

When reloading a form containing checkboxes, the incorrect options are checked
if the form has changed since the reload.

Reproducible: Always

Steps to Reproduce:
1 - Go to the test case that will be uploaded.
2 - Check a record that you whish to delete - Suggesting Record 1<BR>
3 - Hit reload.

Actual Results:  
If you had selected record 1 then after the reload is done, record 2 is now
selected.

Expected Results:  
After the reload, I would expect that nothing is checked if only record 1 was
selected.

I think there is to issues here.
1 - if the page has changed then it is a new page so that data from the form
should be removed from the cache.
2 - If the checkbox value associated the the name is different after the reload
then it should not check the next option.

I checked Bug 46845 and it seems to be related to the reloading of the form
however this one is more related to the refilling the form with cached data.

Comment 1

[Ian]

Attachment: Test case to show the bug.

Comment 2

[Boris 'pi' Piwinger]

Confirming on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1b) Gecko/2002090311.

pi

Comment 3

[Jonas Jørgensen]

AFAICT, this bug

1) is a subset of bug 46845
2) will be fixed by fixing bug 46845
3) cannot be fixed without fixing bug 46845

IMO it's a dupe.

Comment 4

[John Keiser (jkeiser)]

Not a dup, or even dependent, since this happens on back and forward too.  We
may as well save/restore checkboxes by value, we do with <select> and <input
type=radio>.  It is slightly problematic though, due to our save/restore state
system's way of doing things.

Comment 5

[John Keiser (jkeiser)]

Marking dependent on "Move Save/Restore Out of Layout."

Comment 6

[John Keiser (jkeiser)]

I must admit I'm a bit dubious about this, given the memory requirements of it.
 Restoring by value would mean we'd have to store the value of every checkbox on
every page in history in memory.

Comment 7

[csincock]

As mentioned in comment #4, this happens on 'forward' and
'back' too, and is not just limited to forms which have changed.

ie, if you have a checkbox selected in a form,
then leave that form by clicking on a link, then if you
then go 'back' to that page, the checkbox _above_ the
one you picked is selected. The problem described in the original
bug report reports it moving the other way, so care has to be 
given to making sure that fixing one doesn't make the other worse.
Maybe that is what has already happened...

I think this is an important bug to fix, because it is quite common
to partially fill out some form, then go and click on a 'privacy policy'
link or 'terms and conditions' link etc, before submitting the form. If 
the link does not open in a new window or tap, then when you come back
to the form, it looks like it has kept your filled out data, but actually
it has changed the checkboxes.

This could even be the culprit behind all those 'Don't send me spam'
checkboxes in forms I've filled out over the years which automagically
clear themselves when you return to the form despite your name, address etc
being restored OK.

I've just checked, and yes, if there is only one checkbox, it gets cleared on
a 'forward' or 'back' to the page. (at http://mail.yahoo.com/)

For the case where the form has not changed, ie when the user has just gone 
forward/backward, In that case, isn't it likely to be an off-by-one type error,
and easily fixed?
I agree with the reporter that if the page has changed, whether by javascript or
by an actual change in the page source, cached settings should not be used at
all, as it is potentially a totally different form. 

whether 'Reloading' a page should clear form data has been hacked over in bug 
46845, but that discussion seems to have ended up being about the users
'preference'. I think it's clear that if a page has changed (or been reloaded so 
that it has potentially changed), then potentially the form also has changed, 
and you can't just put old values back in to it just because a form
element has the same name, or you'll end up with bugs like the one I discovered
in konqueror once - where I entered the root username and password into a webmin
login form,  then in the process of going back/forward later, the root password
(and other form data) was just just appearing in various text fields and as 
button names. ie as I back/forwarded past that login page again, instead of 
a "Login" button I had a button displaying the root password. 

So anyway, just to sum up, I don't think you need to try this expensive 
restore-by-value method that you've mentioned, as the data shouldn't be 
restored if the form has changed- it should be reset to what the form 
specifies. If the form hasn't changed you can just fix the off-by-one error, 
(if that's what it is).

I have just tried using this page itself to test the problem (it has a 
single checkbox), and after going back and forward again, the checkbox
is still correctly selected. Also, the bugzilla bug query page does not
exhibit the forward/back problem either.

The pages which did not work properly for me were:
 1 checkbox:              http://mail.yahoo.com
 more than one checkbox:  the inbox view of my yahoo mail page.
              sorry I can't think of another test page right now , and 
              phoenix has decided it won't show any page sources.

Comment 8

[Ben Fowler]

Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a6) Gecko/20041215
Firefox/1.0+

Still in. (Very nice testcase, BTW).

Note that Shift Reload produces the correct behaviour.

From an HCI point of view (Comment 7 passim), I don't expect browsers
to get this right, so I am not personally too bothered by it; but it
looks like a pretty important problem.

Comment 9

[Magnus Melin [:mkmelin]]

Still present, Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1b2) Gecko/20060905 BonEcho/2.0b2 ID:2006090504

Comment 10

[Mats Palmgren (inactive)]

So for the form data in the testcase:
<FORM NAME="TEST_FORM">
<INPUT NAME=RECORD TYPE="CHECKBOX" VALUE="1"> 1<BR>
<INPUT NAME=RECORD TYPE="CHECKBOX" VALUE="2"> 2<BR>
<INPUT NAME=RECORD TYPE="CHECKBOX" VALUE="3"> 3<BR>
<INPUT NAME=RECORD TYPE="CHECKBOX" VALUE="4"> 4<BR>
</FORM>

we store the state using the following strings as keys:
'0>130>f>0>0>TEST_FORM>RECORD'
'0>130>f>0>1>TEST_FORM>RECORD'
'0>130>f>0>2>TEST_FORM>RECORD'
'0>130>f>0>3>TEST_FORM>RECORD'

so if the first checkbox is removed it will make the second checkbox
match the first state, etc.  (0,1,2,3 in the key is the child index)

It appears there are no (or very few) duplicates of this bug, so I'm
not sure this is worth messing with.  It can be easily worked around
by the page author by using unique NAMEs for the checkboxes.

Comment 11

[garfield]

This is still a thing in 62.0.3. Just saying …

Comment 12

[garfield]

(In reply to Mats Palmgren (:mats) from comment #10)
> It appears there are no (or very few) duplicates of this bug, so I'm
> not sure this is worth messing with.  It can be easily worked around
> by the page author by using unique NAMEs for the checkboxes.

Actually no. Even with unique names, when reloading a changed form, the next checkbox is checked when the previously checked becomes unavailable!