Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#7823 closed defect (fixed)

dijit.layout.ContentPane and dijit.form.Form interaction produces error

Reported by: mog Owned by: bill
Priority: high Milestone: 1.3
Component: Dijit Version: 1.2.0
Keywords: Cc: ben hockey
Blocked By: Blocking:

Description

Saving all the attached files to one directory then loading test_cp.html will display the following behaviour

When the "change" button is clicked multiple times, this error will be seen in the console:

"Tried to register widget with id==form_ticket but that id is already registered"

In this particular configuration there are no other ill effects, but in a form with more elements, they never load properly.

The attached pages contain a dijit.layout.ContentPane? that contains another dijit.layout.ContentPane? that contains a dijit.form.Form.

If this sounds convoluted, it's because we have a pretty standard layout of a menu pane that loads a center pane. This center pane sometimes has a form with another content pane in it. 5-10 years ago we would have used frames with this configuration.

Attachments (3)

test_cp.html (1.3 KB) - added by mog 11 years ago.
ticket.html (233 bytes) - added by mog 11 years ago.
form.html (93 bytes) - added by mog 11 years ago.

Download all attachments as: .zip

Change History (10)

Changed 11 years ago by mog

Attachment: test_cp.html added

Changed 11 years ago by mog

Attachment: ticket.html added

Changed 11 years ago by mog

Attachment: form.html added

comment:1 Changed 11 years ago by ben hockey

my fix for #7784 fixes this one

comment:2 Changed 11 years ago by bill

Cc: ben hockey added
Component: GeneralDijit
Owner: anonymous deleted

Hi Neonstalwart,

Why does your #7784 fix solve this problem? The change button in this test case is calling setHref(); it isn't manually mucking w/the contents of the ContentPane.

comment:3 Changed 11 years ago by dsnopek

I looked at this case a bunch too. My fix on ticket #7784 will fix this too.

Its because with a nested ContentPane?'s, doing setting.empty() will only destroy the widgets that were parsed and *NOT* their children. So, the 2nd level ContentPane? will be cleaned up, but not its contents.

Both neonstalwarts fixes help, because it means that dijit._Widget.destroyDescents() gets called and will take care of all remaining descendents of the 1st level ContentPane?, which includes the children of the 2nd level ContentPane?.

comment:4 Changed 11 years ago by bill

OK, so the outer ContentPane calls destroy() on the inner ContentPane, but that doesn't destroy the inner ContentPane's descendants.

I need to think about that. Arguably, ContentPane?.destroy() should be destroying ContentPane's descendants (regardless of #5796), since, if the content was set via attr('content') or attr('href'), then ContentPane itself created those subwidgets. That's the current rule in dijit. For example, Tree.destroy() removes all the TreeNodes.

comment:5 Changed 11 years ago by bill

Milestone: tbd1.3
Owner: set to bill
Status: newassigned

comment:6 Changed 11 years ago by bill

Resolution: fixed
Status: assignedclosed

Fixed in [15610]: Add code to ContentPane so it will destroy all widgets inside it's containerNode, even if they were inserted manually by the user. As before, ContentPane will also track things like Menus, even though the domNode is moved to <body>, provided that the Menu was added to the ContentPane via an href or an attr('content', ...).

Also fixed problem where nested ContentPanes wouldn't destroy properly because the outer ContentPane was calling destroy() on the inner ContentPane, and it's contents weren't being destroyed.

Finally, fixed issue with widgetsInTemplates getting destroyed twice by taking advantage of new getDescendants(true) API.

Fixes #7706, #7784, #7823. !strict

Patch is in conjunction with [15607]

comment:7 Changed 11 years ago by bill

(In [16904]) 1. Refactor [15607] (refs #6954, #7550, #7706, #7784, #7823) so that getDescendants() works as in 1.2 again, returning all descendant widgets, even nested widgets that are defined in templates. Expose the new functionality from [15607], to find direct descendants only, into a new _Widget.getChildren() method, rather than as flag to getDescendants().

This is because:

  • [15607] gave getDescendants() a subtle API change: it no longer found widgets inside of a dijit.Declaration (since no containerNode was defined)
  • because of that, Form was broken in that it didn't find form widgets inside of a custom widgets, declared with dijit.Declaration or dojo.declare, which didn't define this.containerNode
  1. Also, rolling back #7819: ContentPane?: missing an addChild() method (refs #7819), because of the backwards compatibility issue for custom widgets that extend ContentPane? and also extend _Container/_Contained.

Summary:

  • getChildren() is now supported by all widgets, and _Container widgets have (as before) an optimized implementation of getChildren() that overrides the one in _Widget.
  • ContentPane? not supporting any _Container methods except getChildren(). Users can set a ContentPane? child by calling attr('value', ...) etc.

!strict

Note: See TracTickets for help on using tickets.