Fixes #9646 - IE7: Cloning of form-elements and changing their names also changes the name of the elements that are cloned. #947

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@markelog
Member
markelog commented Oct 5, 2012

Problem is not limited to only input element and name property (attribute), test case can be reproduced on all elements and all attributes (properties), except for value attribute.

@dmethvin
Member

Wow, if this fixes the problem it would be great! What kind of performance hit are we going to get by having an extra .getAttribute() call for every .setAttribute()? I suppose we are only in this hook on oldIE, but the problem only occurs on IE7, right?

@markelog
Member
markelog commented Nov 2, 2012
.getAttribute() call for every .setAttribute()?

Not exactly on every setAttribute, boolean attributes and specific hooks, like for style attribute would not be affected.

performance hit

for nodeHook it's 30%
for jQuery#attr it's 9%

I suppose we are only in this hook on oldIE

IE8 does not use it.

but the problem only occurs on IE7, right?

Not only that, but this fix deals with very specific edge case, which does not happen very often.

@dmethvin
Member
dmethvin commented Nov 2, 2012

The perf hit is a bit worrisome. Would it be crazy to try to do this in manipulation.js/cloneFixAttributes, since it only happens with cloned elements? In essence we would be eliminating mergeAttributes and copying the attributes ourselves. That will make cloning slower but it won't slow down general attribute retrieval which is much more common.

@dmethvin

Why are these attr gets here? We're not using the result.

Owner

We are not, just like in here and here.

But in order to trigger the error, we have to do that, remember?

@dmethvin dmethvin closed this in 13651f2 Dec 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment