Opened 10 years ago

Closed 10 years ago

#9109 closed defect (worksforme)

#7748 returned in 1.3.0? dojo.clone fails in FF and not IE

Reported by: Phil Bowles Owned by: sjmiles
Priority: high Milestone: tbd
Component: Events Version: 1.3.0
Keywords: clone, connect, event, mouseenter Cc: elatzukin, Adam Peller, James Burke
Blocked By: Blocking:

Description (last modified by Adam Peller)

dojo.clone fails in FF 3.0.8, works in IE 7.0.5730

Testcase is simple:

var newnode=new dijit.form.TextBox();
console.warn("newnode",newnode);
var clone=dojo.clone(newnode);
console.warn("clone",clone);

works 100% as per expected in IE7

in FF, fails thus:

e is undefined
fp()()/code/do.../event.js (line 33)
clone()(Object)/code/do...e/lang.js (line 257)
clone()(function())/code/do...e/lang.js (line 261)
clone()([input#dijit_form_TextBox_0.dijit, "onmouseenter", function(), 1 more... 0=input#dijit_form_TextBox_0.dijit 1=onmouseenter])/code/do...e/lang.js (line 243)
clone()([[input#dijit_form_TextBox_0.dijit, "onmouseenter", function(), 1 more... 0=input#dijit_form_TextBox_0.dijit 1=onmouseenter] 0=[4]])/code/do...e/lang.js (line 243)
clone()([[[input#dijit_form_TextBox_0.dijit, "onmouseenter", function(), 1 more... 0=input#dijit_form_TextBox_0.dijit 1=onmouseenter] 0=[4]], [[input#dijit_form_TextBox_0.dijit, "onmouseleave", function(), 1 more... 0=input#dijit_form_TextBox_0.dijit 1=onmouseleave] 0=[4]], [[input#dijit_form_TextBox_0.dijit, "oninput", function(), 1 more... 0=input#dijit_form_TextBox_0.dijit 1=oninput 3=1] 0=[4]] 0=[1] 1=[1] 2=[1]])/code/do...e/lang.js (line 243)
clone()([Widget dijit.form.TextBox, dijit_form_TextBox_0] _connects=[3] _deferredConnects=Object)

this looks to be the same code (without the webkit additions, which I don't understand) as trac #7748, i.e. its something to do with the special handling of mouseenter events in FF

..either way, I can't clone a node in FF without getting this error!

PS the line numbers in lang.js may be wrong - i put some console.debug output in my source installation to track this bug down

Attachments (1)

cloneNode.html (797 bytes) - added by dante 10 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 10 years ago by dante

Priority: highnormal
severity: blockernormal

you aren't cloning a node in your example, you are attempting to clone an instance of a dijit.form.TextBox? (complex object) ... I don't believe cloning widget instances is a supported behavior of dojo.clone ... you should be able to clone the .domNode no problem, but the events and whatnot aren't likely to be setup properly.

comment:2 Changed 10 years ago by Phil Bowles

From the api docs: "Clones objects (including DOM nodes) and all children. Warning: do not clone cyclic structures." From the DojoCampus? docs: Part of Base Dojo (dojo.js) - Clones anything objects and/or nodes, returning a new anything.

I can't see how any of that states that widget cloning is not supported - quite the opposite in fact, as clone works exactly as expected (and documented) in IE...I get a clone of the textbox, which is what I wanted and what the docs say should happen.

There is definitely a problem with the code mentioned when running under FF.

Also not sure why the status had been changed...IMHO this is a blocker..code that works as per spec in IE fails completely in FF - I can't run the app

comment:3 Changed 10 years ago by dante

Maybe I'm just missing something then ... but is this not the same situation as http://bugs.dojotoolkit.org/ticket/2456

clone() clones nodes, and value based objects. It does not do recursive/circular references at all (which a Widget instance is).

I suppose the documentation should be updated to reflect explicitly cloning of widget objects is not supported. One would expect cloning a widget reference to create a new instance of that reference, but that's simply not how it works (nor has it ever afaik). To do so would require Dojo knowing about Dijit, dojo.clone understanding dijit._Widget/dijit.registry and managing all that.

attached is a test case showing the cloning of a widget domNode reference. Even this introduces problems, as the id's assigned to the widgets by Dijit are cloned as well. When you focus the first [cloned] textbox node, the focus indication is set on the "original/real" instance.

Are you saying this is a regression, and dojo.clone used to clone widgets without problem? If that is the case, the severity should be reset, but I am under the impression this was never supported, and can be fixed by updating the documentation.

Perhaps a dijit.clone() API that would create new instances and handle the ID's/registry/etc would be a nice enhancement, but that kind of code doesn't belong in base.

Changed 10 years ago by dante

Attachment: cloneNode.html added

comment:4 Changed 10 years ago by dante

Cc: elatzukin Adam Peller James Burke added

comment:5 Changed 10 years ago by Adam Peller

Description: modified (diff)

comment:6 Changed 10 years ago by Adam Peller

The culprit appears to be here near the bottom of dojo.clone:

	// Generic objects
	r = new o.constructor(); // specific to dojo.declare()'d classes!

it's cloning the _connect array which (on non IE browsers) has a connect handle with an argument in its constructor:

(function (e) {if (dojo.isFF <= 2) {try {e.relatedTarget.tagName;} catch (e2) {return;}}if (!dojo.isDescendant(e.relatedTarget, node)) {return ofp.call(this, e);}})

The clone code is designed to work for dojo.declare but won't work for objects which require arguments to the constructor. I'm not sure it's possible to clone such objects.

comment:7 Changed 10 years ago by Adam Peller

the recursive problem, iirc, results in an infinite loop. We decided not to fix (#2486) Nodes are cloned by a special case so their recursive references aren't an issue, but if you did hit one, you'd know.

comment:8 Changed 10 years ago by James Burke

Resolution: worksforme
Status: newclosed

The test page works for me now with latest 1.4 code, so closing. elazutkin did changes to clone in the 1.4 timeframe, so it may have been fixed with that.

Note: See TracTickets for help on using tickets.