bug with clone + $$ in IE6/7 #2310

Closed
karolis-k opened this Issue Feb 23, 2012 · 12 comments

Comments

Projects
None yet
6 participants

I clone element without keeping id, but it still can be found with selector using id in IE6 and IE7.

Please find an example here:
http://jsfiddle.net/rmgVM/2/

Just to let you know, I solved my problem by cloning an element this way (because I don't need to keep events or storage in this case):

function softClone(elem) {
    return new Element('div').set('html', elem.clone().outerHTML).getFirst()
}
softClone($('Q1')).inject($('Q1'), 'before')
alert($$('input[id=Q1]').length)

http://jsfiddle.net/Vq5b9/2/

EDIT: it doesn't work in FF...

Owner

ibolmo commented Feb 23, 2012

from your example you're not removing the original element. therefore
you're finding the original element. pls fix

On Thu, Feb 23, 2012 at 3:52 AM, karolis-k <
reply@reply.github.com

wrote:

I clone element without keeping id, but it still can be found with
selector using id in IE6 and IE7.

Please find an example here:
http://jsfiddle.net/rmgVM/


Reply to this email directly or view it on GitHub:
#2310

in IE6/7 it was capturing original element AND the clone (alert = '2').

But I updated it as you wish now and alert is '0' vs '1' in normal browser vs IE6/7

Owner

ibolmo commented Feb 23, 2012

Thanks, will take a look

On Thu, Feb 23, 2012 at 8:29 AM, karolis-k <
reply@reply.github.com

wrote:

in IE6/7 it was capturing original element AND the clone (alert = '2').

But I updated it as you wish now and alert is '0' vs '1' in normal browser
vs IE6/7


Reply to this email directly or view it on GitHub:
#2310 (comment)

Member

DimitarChristoff commented Jun 25, 2012

this looks like a Slick issue rather than anything else.

alert(document.getElements('input#Q1').length); -> 0

input[id=Q1] -> 1

http://jsfiddle.net/rmgVM/7/ - tested on native ie7 - testing this in IE9 in IE7 mode does not produce the bug.

ping @fabiomcosta

http://jsfiddle.net/dimitar/rmgVM/11/ - it looks like the id is correctly removed there. and it's not caused by the invalid markup in the original jsfiddle either.

@karolis-k a few things.

var q1 = $('Q1');
q1.clone().replaces(q1);

// use the correct selector for id:
alert($$('input#Q1').length); // 0
Member

DimitarChristoff commented Jun 25, 2012

sorry it is not Slick. Sizzle also finds 1 element with input[id=Q1] after mootools clones. http://jsfiddle.net/rmgVM/16/

Been looking at it with @kentaromiura and he reckons it looks like it is specific to id and does not fail if you remove the id attr of the copied element. something that has yet to work for me.

Member

kentaromiura commented Jun 25, 2012

I found the culprit in the IE7 cloneNode implementation, I put down a quick fix (http://jsfiddle.net/rmgVM/17/), I need to check if it will pass the tests, clean it up a bit, make it trigger only if ie<8 things like that.

Member

fakedarren commented Jul 28, 2012

@kentaromiura pull request pls :)

@kentaromiura kentaromiura pushed a commit to kentaromiura/mootools-core that referenced this issue Jul 30, 2012

Cristian Carlesso fix bug #2310
IE7 clone node
(http://msdn.microsoft.com/en-us/library/ie/ms536365(v=vs.85).aspx)
have problem when cloning element with id attribute stetted.
5b0f63c

ibolmo referenced this issue Jul 30, 2012

Closed

fix bug #2310 #2387

@arian arian added a commit to arian/mootools-core that referenced this issue Aug 19, 2012

@arian arian Fixes #2310 - Using get/erase/set('id') which uses fix #2083, instead…
… of outerHTML hack.
4cdf46d
Owner

arian commented Aug 19, 2012

Also check out arian/mootools-core@4cdf46d / https://github.com/arian/mootools-core/compare/bug-2310

@kentaromiura your specs seem to pass with my fix too...

Owner

arian commented Aug 19, 2012

Tested this in IE6/IE8.. need to test this in IE7 for those crashes we had...

Owner

arian commented Aug 19, 2012

Ok, doesn't work in IE7... changing clone.removeAttribute (https://github.com/arian/mootools-core/blob/4cdf46d/Source/Element/Element.js#L848) to Element.erase fixes the failing test, but creates some new failures

Owner

ibolmo commented Mar 3, 2014

Moving discussion to the PR.

ibolmo closed this Mar 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment