#9777 closed bug (wontfix)

Opened July 07, 2011 07:22PM UTC

Closed October 22, 2012 05:40PM UTC

.attr() changing wrong element on cloned elements (IE7 only)

Reported by:	matthew.leonhardt@gmail.com	Owned by:	timmywil
Priority:	low	Milestone:	1.next
Component:	manipulation	Version:	1.6.2
Keywords:		Cc:
Blocked by:		Blocking:

Description

The following script highlights the differences. This works as expected in IE8 and FF, or using jQuery <= v1.5.2. It seems restricted to jQuery >= 1.6 and IE7.

Basically, on cloned elements, .attr() seems to update the wrong element.

<html>
  <head>
    <script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1.6.2/jquery.min.js"></script>
    <script type="text/javascript">
    
var curItem = 0;
$(document).ready(function()
{
  $("#newItem").click(function()
  {
    curItem++;
    addItem($("#items"), curItem);
  });
});

function addItem(elem, iter)
{
  var lastRow = elem.find("tr:last");
  var newRow = lastRow.clone(true);

  newRow.find(":input").each(function()
  {
    var newName = $(this).attr("name").replace(/^(.*)_\\d+$/, "$1_" + iter);

    // THIS CHANGES THE WRONG ELEMENT    
    $(this).attr("name", newName);
    
    // THIS WORKS
    //this.name = newName;
    
    $(this).val(newName);
    $("#thisData").text($(this).serialize());
  });
  
  lastRow.after(newRow);
  
  $("#formData").text($("form").serialize());
}

    </script>
    </head>
  <body>
    <form action="#" method="post">
      <table id="items">
        <tr>
          <th>Item</th>
          <td><input type="text" name="item_0" value="item_0"><td>
        </tr>
      </table>
      <input type="button" id="newItem" value="Add Item">
    </form>
    <pre id="thisData"></pre>
    <pre id="formData"></pre>
  </body>
</html>

Attachments (0)

Change History (33)

Changed July 07, 2011 07:37PM UTC by anonymous comment:1

Ugh. Failure to follow directions. Sorry for the code post. Here's the jsFiddle link: http://jsfiddle.net/MWfvw/

Changed July 07, 2011 07:42PM UTC by timmywil comment:2

component:	unfiled → manipulation
owner:	→ matthew.leonhardt@gmail.com
priority:	undecided → low
status:	new → pending

The html shown is invalid. That td needs a closing tag.

This works fine:

http://jsfiddle.net/timmywil/f5Pmg/

Let us know if that fixes the problem.

Changed July 07, 2011 08:24PM UTC by matthew.leonhardt@gmail.com comment:3

status:	pending → new

Thanks for the input, but it doesn't really solve the problem. Updated here: http://jsfiddle.net/MWfvw/6/, but still fails in IE7. The real world scenario involves cloning an entire table with 50+ input fields, so the each() method is unavoidable, I think.

Changed July 07, 2011 09:50PM UTC by timmywil comment:4

_comment0:	http://jsfiddle.net/timmywil/MWfvw/12/ test case with cloning] \ [http://jsfiddle.net/timmywil/MWfvw/16/ test case without cloning] \ \ The right name is being set, but there is some weirdness with the manipulation. → 1310075423535805
status:	new → open

test case with cloning

test case without cloning

The right name is being set, but there is some weirdness with the manipulation.

Changed July 07, 2011 09:51PM UTC by timmywil comment:5

Here's another test case with cloning

Changed July 12, 2011 08:56AM UTC by FND comment:6

_comment0:	I can confirm this with another test case: \ http://jsfiddle.net/wYYUL/ \ https://gist.github.com/1077553 \ \ When replacing ''.attr("name")'' with ''.get(0).name'' (i.e. directly accessing DOM element properties), everything works as expected. → 1310461004691320

I can confirm this with another test case:

http://jsfiddle.net/wYYUL/

https://gist.github.com/1077553

When replacing ''.attr("name")'' with ''.get(0).name'' (and the equivalent for setting values; i.e. directly accessing DOM element properties), everything works as expected.

Changed July 12, 2011 05:15PM UTC by rwaldron comment:7

status:	open → pending

Can this be reduced to simple < 5 line test case?

Changed July 13, 2011 03:21PM UTC by FND comment:8

_comment0:	Reduced test case: \ http://jsfiddle.net/mn7KG/1/ \ \ (Not quite down to five lines, but that's mostly because we don't have ''console.log'' in IE... ) \ \ Note that in regular browsers, each line being reported has one more "x" than the last one - but in IE7, all lines except the first have the same name, suggesting the respective element (or object) is erroneously modified retroactively. \ \ Again, when directly accessing the DOM element's ''.name'' property, the buggy behavior disappears. → 1310570629986361
_comment1:	Reduced test case: \ http://jsfiddle.net/mn7KG/2/ \ \ (Not quite down to five lines, but that's mostly because we don't have ''console.log'' in IE... ) \ \ Note that in regular browsers, each line being reported has one more "x" than the last one - but in IE7, all lines except the first have the same name, suggesting the respective element (or object) is erroneously modified retroactively. \ \ Again, when directly accessing the DOM element's ''.name'' property, the buggy behavior disappears. → 1310570698411258

_comment0:

Reduced test case: \ http://jsfiddle.net/mn7KG/1/ \ \ (Not quite down to five lines, but that's mostly because we don't have ''console.log'' in IE... ) \ \ Note that in regular browsers, each line being reported has one more "x" than the last one - but in IE7, all lines except the first have the same name, suggesting the respective element (or object) is erroneously modified retroactively. \ \ Again, when directly accessing the DOM element's ''.name'' property, the buggy behavior disappears. → 1310570629986361

_comment1:

Reduced test case: \ http://jsfiddle.net/mn7KG/2/ \ \ (Not quite down to five lines, but that's mostly because we don't have ''console.log'' in IE... ) \ \ Note that in regular browsers, each line being reported has one more "x" than the last one - but in IE7, all lines except the first have the same name, suggesting the respective element (or object) is erroneously modified retroactively. \ \ Again, when directly accessing the DOM element's ''.name'' property, the buggy behavior disappears. → 1310570698411258

Reduced test case:

http://jsfiddle.net/mn7KG/2/

(Not quite down to five lines, but that's mostly because we don't have ''console.log'' in IE... )

Note that in regular browsers, each line being reported has one more "x" than the previous one - but in IE7, all lines except the first have the same name, suggesting the respective element (or object) is erroneously modified retroactively.

Again, when directly accessing the DOM element's ''.name'' property, the buggy behavior disappears.

Changed July 28, 2011 08:01AM UTC by trac-o-bot comment:9

resolution:	→ invalid
status:	pending → closed

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

Changed July 29, 2011 06:29AM UTC by FND comment:10

This issue is still relevant - see reduced test case above.

Please reopen.

Changed July 29, 2011 02:35PM UTC by dmethvin comment:11

resolution:	invalid
status:	closed → reopened

Confirmed. That retroactive modification is bizarre. Here is a screen shot: http://imgur.com/Ztfv0

I tried changing to use .prop() and it showed strange and incorrect but different behavior.

The reason the ticket was auto-closed is that the original reporter did not reply.

Changed July 29, 2011 02:36PM UTC by dmethvin comment:12

component:	manipulation → attributes
status:	reopened → open

Changed September 24, 2011 01:39PM UTC by Martin comment:13

I also stumbled upon this today. Complete short test case for IE7 is already posted here:

http://stackoverflow.com/questions/7539354/jquery-1-6-4-cloning-problems-in-internet-explorer-7

Changed September 24, 2011 01:56PM UTC by rwaldron comment:14

owner:	matthew.leonhardt@gmail.com → timmywil
status:	open → assigned

Changed September 24, 2011 07:07PM UTC by timmywil comment:15

_comment0:	It seems setting any attribute node on a clone sets all previous clones as well. It has a weird affect on the property as well. \ \ set name => http://jsfiddle.net/timmywil/mn7KG/19/ \ set id => http://jsfiddle.net/timmywil/mn7KG/21/ \ using property => http://jsfiddle.net/timmywil/mn7KG/28/ → 1316891264914294

It seems setting any attribute node on a clone sets all previous clones as well. It has a weird affect on the property as well.

set name => http://jsfiddle.net/timmywil/mn7KG/19/

set id => http://jsfiddle.net/timmywil/mn7KG/21/

using property => http://jsfiddle.net/timmywil/mn7KG/28/

Changed September 24, 2011 07:57PM UTC by rwaldron comment:16

What about a feature detection for this? That way we could create an element instead of cloning (only for cases meeting the detection condition)

Changed October 07, 2011 08:46PM UTC by timmywil comment:17

#10453 is a duplicate of this ticket.

Changed December 14, 2011 04:17AM UTC by dmethvin comment:18

Hmmm, perhaps this is happening because the attribute nodes are not being copied but instead are referenced from both elements? Also this somewhat newish version of the docs for .mergeAttributes() gives some more info on the API and its poorly explained Boolean argument.

http://msdn.microsoft.com/en-us/library/ms536614%28v=vs.85%29.aspx

Notice the docs specifically mention the NAME and ID attributes/properties which seem to be the problems.

Changed December 15, 2011 04:24PM UTC by timmywil comment:19

Looking further at this issue, I tried an html5 element vs a div and found that the inner/outerHTML method for cloning does not have this issue.

http://jsfiddle.net/timmywil/mn7KG/30/

http://jsfiddle.net/timmywil/mn7KG/31/

rwaldron suggested using setAttribute, but that sets the property, which has the same issue as the third fiddle I've posted above:

http://jsfiddle.net/timmywil/mn7KG/32/

This is about the way we clone.

Changed January 24, 2012 07:22PM UTC by FND comment:20

I'm not sure whether this is relevant (or even new), but it appears IE7's ''innerHTML'' includes DOM properties as HTML attributes: http://jsbin.com/ejurey/2

Changed January 30, 2012 07:45PM UTC by anonymous comment:21

I too found this issue (affects jQuery 1.7.0 and 1.7.1 (not sure about before that)).

Here was my simple jsFiddle: http://jsfiddle.net/f2cxc/10/ the issue as far as I can tell is actually that there is no clear separation in IE7 from the original, and the cloned selection set.

My suspicion is that internally there is a reference to the old set that is not properly being broken in IE7 (e.g. this is still likely an IE7 bug under the covers)

Changed January 30, 2012 08:27PM UTC by scunliffe comment:22

Hi I'm "scunliffe" (I was the anonymous commenter above (didn't realize I wasn't logged in)... I ran some further tests and it appears that the behavior worked fine back in jQuery 1.5.2. I think it would be prudent to run a diff between how .clone() and/or .attr() changed between 1.5.2 and 1.6.4.

Again, here's the jsFiddle that FAILS in IE6/IE7 on jQuery 1.6.4,1.7.1

http://jsfiddle.net/f2cxc/10/

Yet with the same code WORKS fine in IE6/IE7 on jQuery 1.5.2

http://jsfiddle.net/f2cxc/12/

PS I have access to IE6/IE7/IE8 should anyone have a patch they'd like an extra set of eyes to test/check.

Changed January 30, 2012 08:48PM UTC by timmywil comment:23

component:	attributes → manipulation

@scunliffe: Yes, it's because we stopped using innerHTML to clone in IE and switched to cloneNode. We'd like to keep using cloneNode if we can.

Changed January 31, 2012 03:38PM UTC by scunliffe comment:24

Understood. Is it a matter then of determining which parts of cloneNode for IE6/IE7 needs "polyfills" of some sort to maintain compatibility?

I'm not sure if/when IE fixed their issues with cloneNode but I do recall that at one point (again due to related IE bugs with readonly attributes) the cloneNode method was effectively useless as all copies would auto-inherit the "name" attributes from their originator.

(this is an old report, but seems to suggest this might be impossible in IE6/IE7)

http://webbugtrack.blogspot.com/2008/03/bug-199-cant-clone-form-element-in-ie.html

Combined with IE's cluster muck with ID's and NAME attributes sharing the same namespace I don't know if there even is a way to coerce IE into behaving using .cloneNode in IE6/IE7.

Changed March 14, 2012 05:28PM UTC by Stephen Cunliffe <stephen.cunliffe@gmail.com> comment:25

Is there a workaround in place for this? or a fix coming? If not, can we update the documentation http://docs.jquery.com/Clone to indicate that there are issues in IE (below IE8) if you plan to manipulate the cloned element(s)... specifically the name and id attributes.

Changed March 30, 2012 06:46PM UTC by teresa@twohemisphere.com comment:26

I noticed the priority on this issue is low, but this change has introduced a significant bug into my codebase. Is there a possibility for a patch or workaround anytime soon?

Changed March 30, 2012 06:57PM UTC by timmywil comment:27

The only way I can think of to fix this is to use innerHTML to clone all nodes in IE 6/7. That may not be possible, at least for a while. It is not an issue with attr or prop, but prop will give you slightly more consistency in the values. It will still not be entirely correct, but may be enough for regular use cases.

Changed March 30, 2012 07:03PM UTC by dmethvin comment:28

Also note that cloning via .innerHTML is the approach we abandoned as of version 1.5 due to its drawbacks, so unless it was some more complex hybrid of the two approaches we might just be bringing back all those older bugs.

https://github.com/jquery/jquery/commit/4fae75d575b20d887e4a273c7991c55f8821a62c

Changed July 23, 2012 11:00AM UTC by regisbsbittencourt@gmail.com comment:29

Could we raise it to medium at least? it looks like regression of the 1.7.x branch.

Changed July 23, 2012 01:28PM UTC by dmethvin comment:30

The priority DOESN'T MATTER if nobody has a solution.

Changed July 23, 2012 01:42PM UTC by anonymous comment:31

I would say that reverting back to copying the html is the solution. I'm using this workaround now.

Changed July 23, 2012 02:03PM UTC by dmethvin comment:32

That just opens up the old bugs that existed when we used to copy the html. Since then we've started supporting HTML5 elements in oldIE for example, and I suspect that wouldn't fly in the old code without additional work.

Patches welcome though.

Changed October 22, 2012 05:40PM UTC by timmywil comment:33

resolution:	→ wontfix
status:	assigned → closed

I don't think we're going to go back to innerHTML for cloning, which is what is required to fix this.

Side navigation