Opened 9 years ago

Closed 9 years ago

#11906 closed defect (fixed)

Incorrect NodeList.initialization of ContentPanes

Reported by: Katie Vance Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.5
Keywords: Cc: Douglas Hays
Blocked By: Blocking:

Description (last modified by bill)

This is a regression post 1.5 (only and issue in 1.6).

When using NodeList.instantiate for ContentPanes, if the NodeList contains more than 1 node, then all the nodes but the very first node will lose it's original content.

This can be seen with test_instantiate.html (Look at the tab content panes).

The problem is a regression, caused by changeset [22835]. In that changeset, this constructor was added to ContentPane:

constructor: function(params, srcNodeRef){
// Convert a srcNodeRef argument into a content parameter, so that
//the original contents are
// processed in the same way as contents set via set("content", ...)
if(srcNodeRef && !("href" in params) && !("content" in params)){
    var df = dojo.doc.createDocumentFragment();
    params.content = df;

The problem is the instantiate code invokes this constructor multiple times (once for each node in the NodeList) with the same properties list. The list is not cloned so it's not expecting that the constructor and/or widget create methods will modify the properties object. But the new ContentPane constructor does modify the properties object, therefore, causing the next ContentPanes that are created to have the wrong content. (The properties object is referred to as params in the constructor method above.)

Change History (4)

comment:1 Changed 9 years ago by Katie Vance

Description: modified (diff)

comment:2 Changed 9 years ago by Katie Vance

An obvious solution here would be to clone the properties object within NodeList?.instantiate() so that it doesn't matter if the widget constructor/create methods modify the parameter list. However, will doing so add significantly more performance time to the instantiate method?

comment:3 Changed 9 years ago by bill

Description: modified (diff)

Good catch. I suppose that alternately ContentPane could override create() to define a new params object, and then call this.inherited(arguments, [params, srcNodeRef]). That would be instead of having the constructor() method.

comment:4 Changed 9 years ago by bill

Resolution: fixed
Status: newclosed

(In [23120]) Avoid modifying original params object since that breaks NodeList instantiation. Fixes #11906, refs #2056 !strict.

Note: See TracTickets for help on using tickets.