1322455 - Clone of unchecked input becomes checked if the node has the checked attribute

Status: VERIFIED FIXED

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

Description

[Luke]

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20161202091213

Steps to reproduce:

I checked the serialized value of checkbox that had been unchecked:

As a simple example:
http://jsfiddle.net/0h0x9wLz/
Check the checkbox and it should alert alternating "" (no check) and "Checkboxvalue=on"

Unfortunately whenever an originally-checked checkbox is unchecked, this still acts like it is checked:
alert($('form').clone().serialize()); always alerts the same thing as you check it and uncheck it.




Actual results:

Rather than unchecking, a clone of the checkbox is checked as it has the checked=checked value.

On some Firefox system it is always showing checked in this example - Chrome and some Firefox installs seem to switch fine however.


Expected results:

It should probably do the same as in other browsers (and some versions of Firefox) and switch the checked value.

Comment 1

[Emil Pasca [:emilpasca]]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0

I have tested this issue on Windows 10 x64 with the latest Nightly (53.0a1-20161212030206) and managed to reproduce it.
When opening the URL provided in the description and checking/unchecking the checkbox the same message is displayed in the alert window "Checkboxvalue=on".

On the latest Firefox release (50.0) the issue is not reproducible.
I've performed a reggression, below are the results:
Last good revision: 18ff1472dae91c2b75e0ca843d52ad9a34c438cf
First bad revision: aadb57a9ddf83910a604f6be3b787fee661249db
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=18ff1472dae91c2b75e0ca843d52ad9a34c438cf&tochange=aadb57a9ddf83910a604f6be3b787fee661249db

Looks like the following bug has the changes which introduced the regression:
https://bugzilla.mozilla.org/show_bug.cgi?id=1310865

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

[Tracking Requested - why for this release]:
Bad regression



Aryeh is away, so perhaps hsivonen you could take a look at the regression.

Comment 3

[Henri Sivonen (:hsivonen)]

Sigh. So deferring the uplift of bug 1310865 still didn't give it enough bake time to discover this. :-(

I'll take a look.

Comment 4

[Henri Sivonen (:hsivonen)]

Basic-JS test case:
https://hsivonen.com/test/moz/clone-checked.html

Comment 5

[Julien Cristau [:jcristau]]

tracking this recent regression for 51 and up

Comment 6

[Henri Sivonen (:hsivonen)]

Attachment: When cloning an <input>, avoid re-initializing checkedness in DoneCreatingElement

MozReview-Commit-ID: 1QxpiX02V0G

Comment 7

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dddce424dad

Comment 8

[Henri Sivonen (:hsivonen)]

Attachment: When cloning an <input>, avoid re-initializing checkedness in DoneCreatingElement

MozReview-Commit-ID: JbrpGAoB5tG

Comment 9

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8805327729b5

Comment 10

[Henri Sivonen (:hsivonen)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bddca3e98820

Comment 11

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1322455 - When cloning an <input>, avoid re-initializing checkedness in DoneCreatingElement. .

Review commit: https://reviewboard.mozilla.org/r/98866/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/98866/

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8818962 [details]
Bug 1322455 - When cloning an <input>, avoid re-initializing checkedness in DoneCreatingElement. .

https://reviewboard.mozilla.org/r/98866/#review99160

Following various flags in HTMLInputElement has become rather hard. I wonder how to improve that. Oh well, not about this bug.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5bb140e08339
When cloning an <input>, avoid re-initializing checkedness in DoneCreatingElement. r=smaug.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5bb140e08339

Comment 15

[Henri Sivonen (:hsivonen)]

Comment on attachment 8818962 [details]
Bug 1322455 - When cloning an <input>, avoid re-initializing checkedness in DoneCreatingElement. .

Approval Request Comment
> [Feature/Bug causing the regression]:

Bug 1310865.

> [User impact if declined]:

Pages that clone forms with checkboxes or radiobuttons may misbehave by getting the checkedness state wrong.

> [Is this code covered by automated tests?]:

Yes.

> [Has the fix been verified in Nightly?]:

Yes.

> [Needs manual test from QE? If yes, steps to reproduce]: 

No need, but the steps are:
 1) Load https://hsivonen.com/test/moz/clone-checked.html
 2) Check the first box.
 3) Uncheck the second box.
 4) Click the button.
 5) Ensure the third box is checked.
 6) Ensure the fourth box is unchecked.

> [List of other uplifts needed for the feature/fix]:

None.

> [Is the change risky?]:

Relatively unrisky.

> [Why is the change risky/not risky?]:

The patch flips one boolean which is checked in only one place, so the impact should be very constrained.

> [String changes made/needed]:

None.

Comment 16

[Gerry Chang [:gchang]]

Hi :emilpasca,
Could you help verify if this issue is fixed as expected on the latest Nightly build? Thanks!

Comment 17

[Gerry Chang [:gchang]]

Comment on attachment 8818962 [details]
Bug 1322455 - When cloning an <input>, avoid re-initializing checkedness in DoneCreatingElement. .

Fix a regression. Beta51+ and Aurora52+. Should be in 51 beta 9.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2c7ec3a0c964

Comment 19

[Carsten Book [:Tomcat]]

needs rebasing for beta

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/7509b77d7d35

Comment 21

[Iulia Cristescu, QA [:JuliaC] (away, please needinfo? [e-mail])]

I managed to reproduce the issue on 53.0a1 (2016-12-07). The bug is verified fixed on latest Nightly 53.0a1 (2016-12-28), latest Aurora 52.0a2 (2016-12-29) and on 51.0b10 (20161222080852), using Windows 10 x64, Ubuntu 14.04 x86 and Mac OS X 10.11.6.