Opened 13 years ago

Closed 13 years ago

#2724 closed defect (fixed)

Widget.js missing dojo.dom require.

Reported by: guest Owned by: bill
Priority: high Milestone: 0.9beta
Component: Dijit Version:
Keywords: dijit Cc:
Blocked By: Blocking:

Description

Dijit's dijit.base.Widget contains a method that cleans up HTML nodes, destroyRendering. It twice calls dojo.dom.destroyNode, but neither call actually succeeds because dojo.dom hasn't been required by Widget. Errors are hidden because the exception thrown is squelched, and it seems that most currently built widgets get this working by including, either directly or indirectly, dojo.dom.

Attachments (2)

localtests.zip (2.6 KB) - added by guest 13 years ago.
Tests to replicate the error.
Widget.js.patch (461 bytes) - added by guest 13 years ago.
Simple patch. Adds a dojo.require( "dojo.dom" ) statement to the top of the file.

Download all attachments as: .zip

Change History (6)

Changed 13 years ago by guest

Attachment: localtests.zip added

Tests to replicate the error.

comment:1 Changed 13 years ago by guest

Unzipping the tests archive you'll see three files. This should be done one level below the dijit folder, so you have the path dijit/localtests. You can then run testCheckbox.html to see the destroyNode function working because dijit.form.Checkbox indirectly imports dojo.dom. However, testSimpleWidget.html does not delete the widget's HTML nodes because it does not itself import dojo.dom.

Changed 13 years ago by guest

Attachment: Widget.js.patch added

Simple patch. Adds a dojo.require( "dojo.dom" ) statement to the top of the file.

comment:2 Changed 13 years ago by guest

Heh... forgot to mention I have submitted a CCLA under the corporate name Per-Se (though our name has changed to McKesson?) and the personal name Kelly Walker.

-Clay | clayton.carpenter@…

comment:3 Changed 13 years ago by Adam Peller

This problem (sort of) goes away with the 0.9 port anyways, since dojo.dom goes away and most of the methods are provided by base. Interestingly, destroyNode is one of the methods which does not yet exist in 0.9, so we'll have to take a hard look at that.

Rather than mark 'wontfix' though, what really concerns me is exceptions being squelched. We should try to avoid that.

comment:4 Changed 13 years ago by bill

Component: WidgetsDijit
Resolution: fixed
Status: newclosed

I checked the code. Like peller said the dojo.dom calls have been replaced. The lines you are talking about are now commented out:

//			dojo.dom.destroyNode(this.domNode);
//PORT #2931

... and I don't see any squelched exceptions anymore. I think we took those try/catch blocks out a few weeks ago.

Note: See TracTickets for help on using tickets.