259486 - Setting checkbox.disabled to false when it already is false clears the value for the checkbox when reloading

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P2)

Description

[Mattias Holmlund]

User-Agent:       Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.7) Gecko/20040614 Firefox/0.9
Build Identifier: Mozilla/5.0 (Windows; U; WinNT4.0; en-US; rv:1.7) Gecko/20040614 Firefox/0.9

We have a webpage with a input checkbox that is checked via HTML. In the
onLoad-handler, we set checkbox.disabled to false even though it already is
false. When the page is loaded the first time the checkbox appears checked, but
if you reload the page, the checkbox is unchecked.

This problem has been found in Firefox 0.9 on Windows NT, Mozilla 1.6, 1.7 and
1.8 Alpha as well as Mozilla 1.6 from Debian testing on Linux. 

Reproducible: Always
Steps to Reproduce:
1. Load the attached testcase (coming up)
2. Verify that the checkbox is checked.
3. Press the reload button

Actual Results:  
4. The checkbox is no longer checked

Expected Results:  
4. The checkbox should still be checked.

This bug has the potential to cause dataloss, since if you do submit without
noticing that the checkbox has been unchecked, bad things can happen.

Comment 1

[Mattias Holmlund]

Attachment: Testcase as described in the original bugreport

Comment 2

[Mattias Holmlund]

Attachment: Variant of the previous attachment that validates correctly

The original testcase is not valid HTML. This one is. There is no difference in
behaviour from the original testcase.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The problem here is that the checkbox doesn't set its "checked" state if state
was restored.  In this case state was restored because there was "disabled"
state.  But the checkbox doesn't save its checked state in the history if its
checked value is the default.  So we never end up setting it checked.

Possible options here:

1)  Save the checked value no matter what (undoing part of the fix for bug
    62713)
2)  Save the checked value if it's changed or if we're storing something else in
    the state.
3)  Redefine the return value of RestoreState() to indicate something about
    whether "checked" was restored...

#3 is probably the least intrusive and least memory-intensive, but will we ever
care about getting other information out through that return value?

jst?  Thoughts?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 159447 [details] [diff] [review]
Patch

jst, would you take a look?

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 159447 [details] [diff] [review]
Patch

r+sr=jst

Comment 7

[Boris Zbarsky [:bzbarsky]]

Fixed for 1.8a4.