Opened 12 years ago

Closed 9 years ago

#4980 closed defect (fixed)

ContentPane setContent() complaining about creating duplicate popup widgets

Reported by: gid@… Owned by: bill
Priority: high Milestone: 1.3
Component: Dijit Version: 1.0
Keywords: Cc: gid@…
Blocked By: Blocking:

Description (last modified by bill)

I have an application that uses a ContentPane? to replace the majority of the content of the site. Recently when upgrading from 0.9-release to the 1.0 beta I'm getting errors like this: "Couldn't create widgets in djRightPane Error: Tried to register widget with id=SearchHelp? but that id is already registered."

This happens when clicking on the various button on the site that switches the content. Basically what's happening is if I call setContent() with the html that contains the SearchHelp? widget, then swap out the content without something else, and then switch back to the first html with the SearchHelp? widget, it's complaining that SearchHelp? is already defined, this seems to make setContent() not quite as useful as it can no longer redefine an widget with the same id.

Shouldn't all the widgets contained in the ContentPane? go out of scope when setContent() is called so they can be redfined by the new html? Was something to this nature changed between 0.9 and 1.0 beta? Or maybe is this a hidden option somewhere that can alter the behavior of redefining widgets?

I realize this is kind of a vague description, I can try to cut up a quick proof of concept if need be. I'm using a custom build from svn r11285 if that makes a difference.

ttrenka reply on the forums: It shouldn't.

One of the main complaints about Dijits between 0.9 and now is that it was not preserving the attributes of HTML nodes passed to it; that was changed so that it would. Odds are, this is a side effect of what you're trying to do.

If you could file a ticket at trac.dojotoolkit.org (log in as guest/guest) with a description of the issue (probably you can just paste this forum post in), that'd be awesome.

Change History (14)

comment:1 Changed 12 years ago by bill

Yeah, we need a testcase. setContent() calls _setContent() which calls destroyRecursive(), so all the old widgets should be getting destroyed.

comment:2 Changed 12 years ago by guest

test case: http://pimpbot.gifpaste.net/~gid/test/contentPane.html

click on any one of the links twice

comment:3 Changed 12 years ago by bill

Milestone: 1.1
Reporter: changed from guest to gid@…
Summary: ContentPane setContent() complaining about creating duplicate widgetsContentPane setContent() complaining about creating duplicate popup widgets

Hi, thanks for the test case. This is basically another manifestation of #4902. Dialog::postCreate() does:

dojo.body().appendChild(this.domNode);

which moves the Dialog's dom node from inside the ContentPane? to outside of it, thus ContentPane::destroyDescendants() doesn't find it.

I guess that's a bad design but not sure how to fix it. Maybe leave some stub pointer node where Dialog.domNode originally was, and/or only move the dialog node while the dialog is being displayed.

As for now it means that ContentPanes? (or anything that derives from them) have problems containing popup widgets.

comment:4 Changed 12 years ago by guest

Hi, You guys went way over my head with some of your tech talk so, I'm sorry if this is a dumb question. I'm on .9, but when testing 1.0, I'm seeing the same "already registered" problem as a lot of people. I see you have this ticket scheduled for 1.1. I'm trying to figure out if I should upgrade to 1.0 and go through and try to figure out how I can fix these "already registered" problems or if perhaps these problems might disappear for me sometime around 1.1. I don't know if you need more detail, but does this sound like a safe bet? Or do I need to bite the bullet and figure out how to fix them on my end?

Thanks

comment:5 Changed 12 years ago by bill

The problems will disappear for 1.1, and I guess fixing it for 1.0 isn't so easy, so you might want to wait to upgrade. It's up to you.

comment:6 Changed 12 years ago by bill

Same root cause as #3300.

comment:7 Changed 12 years ago by bill

Owner: set to bill

comment:8 Changed 12 years ago by bill

Milestone: 1.11.3

comment:9 Changed 11 years ago by bill

Description: modified (diff)
Milestone: 1.31.2
Owner: changed from bill to Sam Foster

Looks like this will be fixed by sfoster's conversion of ContentPane to use dojo.html.

comment:10 Changed 11 years ago by Sam Foster

(In [14987]) Refs #5647 Refs #2056 Refs #4980

Refactored _setContent to use dojo.html._ContentSetter

  • a dojo.html._ContentSetter instance is created and stored on first use (at this._contentSetter)
  • each subsequent call to _setContent remixes in the config properties
  • _ContentSetter.empty() is called implicitly when you call this._contentSetter.set() - this will destroy any widgets from a previous set operation - including Menus, Dialogs etc, as the contentSetter.parseResults array keep widget references. This is partly redundant with the destroyDescendents() call in _setContent(), but catches this corner case. It does not fix the case where widgets have been created by the parser from inlined, initial content
  • added a few unit tests to the test page to verify this behavior

!strict

comment:11 Changed 11 years ago by bill

Milestone: 1.21.3
Owner: changed from Sam Foster to bill

The actual problem in the test case above is fixed, but as Sam said there's still a problem case with widget declarations inlined into the original page. See comments in #2056.

comment:12 Changed 11 years ago by bill

See also umbrella ticket #6954.

comment:13 Changed 11 years ago by bill

Milestone: 1.32.0

comment:14 Changed 9 years ago by bill

Milestone: 2.01.3
Resolution: fixed
Status: newclosed

Marking as fixed since the test case is fixed, and remaining issue is listed in #2056.

Note: See TracTickets for help on using tickets.