Clone does not correctly copy checked state in IE10

[chatfielddaniel@…]

The bug is with IE not jQuery but viewing the source shows that a similar problem was in webkit and jQuery fixed the browser bug.

See: ​http://codepen.io/danielchatfield/pen/DxzKv

(I know that is native javaScript and not jQuery but it demonstrates the problem that needs to be corrected)

[dmethvin]

Can you provide a jQuery example? Our feature detect for the same Chrome problem should fix it I'd think, but if you've found otherwise we'd like to fix it.

[danielchatfield]

Yep:

​http://codepen.io/danielchatfield/pen/sDwJI

If you uncheck the checkbox and click clone the new checkbox should also be unchecked but in IE10 (and maybe other versions) it is checked.

[danielchatfield]

Also - how do I take ownership of this ticket? It is owned by my email address but not my actual account.

[dmethvin]

how do I take ownership of this ticket?

Normally you'd be the owner if you were planning to submit a patch. Did you want to do that?

As far as the bug goes, it happens in IE9 as well. May happen in oldIE as well but codepen won't run in those. Also i can't get codepen to change the version of jquery in IE9, it will do so in other browsers. My goal was to see whether this is a regression, but codepen isn't cooperating.

Working jsfiddle: ​http://jsfiddle.net/TBEXk/

Does not seem to be a regression. In fact it's a dup of #10550. I'm not sure why that was closed wontfix, looking at the trail, although it's an ambiguous case for sure. The checked property is false but the checked attribute still contains checked. But like you say, we "fix" a similar Chrome situation.

If you'd like to submit a patch on #10550 or look at ​https://github.com/jquery/jquery/pull/558 to see if it can be resurrected, we can consider it.

[dmethvin]

Duplicate of #10550.

[danielchatfield]

Thanks for that - I currently work around it by checking whether it is checked using .is(':checked') before the clone and then making sure the checked attribute is either removed or added depending on its previous state. I imagine that would be far too expensive to just do to all clones.

[dmethvin]

Oh waitaminit, I remember why this was wontfixed. The problem is on the appending of the DOM elements, not the clone. There is no easy central place in jQuery to fix this. You can monkey with the attribute but that produces HTML that isn't the same as the input, which brings its own set of problems.

[danielchatfield]

Just submitted a Pull request that fixes it:

The fix was already in place for IE6-8 but the test that fired the fix incorrectly passed in IE9, I have altered the test so that now it fails in IE9, IE10.

​https://github.com/jquery/jquery/pull/871

[Rick Waldron]

I just commented on the patch - you didn't change any tests, you changed code in "support.js", which you don't need to do.

[danielchatfield]

Hi,

I'll add a test but there was an issue with the code in support.js:

jQuery.support.noCloneCheck was false in IE9 and IE10 even though IE9 and IE10 should have failed this.

[Rick Waldron]

Gotcha.

Either way, there still needs to be a test added to /test/unit/manipulation.js that supports this change (and it all needs to pass in IE6-8 as well)

[danielchatfield]

I have never written a test before so I'm just working out how to do it - where abouts in that file should I add the test? The bottom?

[Rick Waldron]

All of the correct values are present when I run this in IE9:

​http://jsfiddle.net/rwaldron/CCaBQ/

"checked" is a property and is set correctly, attributes !== properties

[danielchatfield]

Replying to rwaldron:

All of the correct values are present when I run this in IE9:

​http://jsfiddle.net/rwaldron/CCaBQ/

"checked" is a property and is set correctly, attributes !== properties

The problem is when you clone a checkbox where the checked attribute is set and the checked property is false. On the cloned element the checked property will be true in IE9.

[Rick Waldron]

​http://jsfiddle.net/rwaldron/CCaBQ/

Open that in any browser (try Chrome or Firefox). checked="checked", checked="true", checked="false"... these all mean the same thing; if the "checked" (boolean) attribute exists, then the box is checked, no matter what — it's very existence causes this.

[danielchatfield]

Let me explain:

I have a form with an input which is checked by default (because the checked attribute is set). I uncheck it (by clicking on it) - this does NOT remove the checked attribute (check the chrome inspector), but rather sets the checked property to false. If you clone this element in chrome you get another checkbox which has the checked attribute but the checked property is false (so it is not checked) - this is what you would expect. In IE however it either fails to copy across the checked property or some event gets fired that shouldn't which means it gets overwritten because of the presence of the attribute. I realise it is counter intuitive because the selector [checked="checked"] selects the checked one but at a DOM level that just isn't what is happening (hence why when you clone an unchecked checkbox it will have the checked attribute if the original one did)

The presence of a checked attribute does not actually mean the input is checked - it means it will be checked by default. If you add the checked attribute to an element the browser will, in accordance with the spec, set the checked property to true.

If you give me 2 minutes I'm almost ready to submit another PULL request - this time with a test.

[Rick Waldron]

No rush, I'm just trying fully understand your issue so I can help

[danielchatfield]

I'm struggling to make a test that is working. Could you please look at this:

​http://jsfiddle.net/bKKGj/2/

Clicking clone should create a copy of the checkbox (and be unchecked), this works in all browsers except IE9, IE10 where the copy is checked. I'm not sure I'm doing the tests right because everything is passing.

[danielchatfield]

I am at a loss here - I really don't know why this test isn't working.

See: ​http://jsfiddle.net/bKKGj/8/

All browser except IE will return true (with current jQuery). And all including IE with my changes. However When I wrap that in a test like so:

test("checked state is cloned with clone()", function(){
	elem = jQuery.parseHTML('<input type="checkbox" checked="checked"/>')[0];
	elem.checked = false;
	result = !jQuery(elem).clone().attr('id','clone')[0].checked;
	ok( result, 'Checked state correctly cloned' );
});

IE passes it with jQuery 1.7.2 - what is wrong?

[danielchatfield]

STUPID ME - after removing my changes I didn't rebuild it.

All was working after all.

[danielchatfield]

And again: ​https://github.com/jquery/jquery/pull/873

[Rick Waldron]

Can we get a confirmation -- is this different from ​http://bugs.jquery.com/ticket/12132

[danielchatfield]

Yep - it is different

[Rick Waldron]

Please review the discussion on the PR as well ​https://github.com/jquery/jquery/pull/870

[danielchatfield]

Well I can confirm that with that change the test that I added would still fail. They might be related but neither my change or that one would fix both problems so one would assume they are isolated problems.

Now that I have studied the source a bit more I know a little more about the problem:

In IE6-8 when a checkbox or radio was cloned the checked state was not copied. To fix this noCloneCheck was added to jQuery.support, and cloneFixAttributes was modified to make sure checked state was correctly cloned. In IE9 this was half fixed - it does correctly carry across the checked property however if the checked attribute exists on the source element then some browser event would get fired (which shouldn't) that would incorrectly set the checked property to true. The existing test does not pick this up because until ie9 a browser either correctly cloned it all the time or never, I have modified the test so that rather than checking whether a 'true' checked value is cloned it tests whether a 'false' one is; as ie6-8 will still fail this and now so will ie9 and ie10. I then modified the code that was designed to carry across the checked state so that it clones it whether it is checked or not.

[Daniel Chatfield]

Fix #12127. IE9/10 checks fall off the box on clone. Close gh-873.

Changeset: 569d064fc93459695cb6eb6fd09e5ba3fda62f03

[Dave Methvin]

Revert "Fix #12127. IE9/10 checks fall off the box on clone. Close gh-873."

This reverts commit 569d064fc93459695cb6eb6fd09e5ba3fda62f03.

Causing test fails in Safari, IE6, and IE7.

Changeset: de213be3728a339e1f2b98712a0869f8afb9da67

[dmethvin]

Test fails: ​http://swarm.jquery.org/job/377

[dmethvin]

We'll need to find a solution to those test fails, still hoping to get this into 1.8 final.

[danielchatfield]

Got it this time - didn't actually need to alter support.js

​https://github.com/jquery/jquery/pull/875

[Daniel Chatfield]

Fix #12127, fer real. IE9/10 check state on clone. Close gh-875.

Changeset: 155855b2a9bd95219871210ae7dcacd2a5f7e117