#12436 closed defect (fixed)
[patch][cla][dijit.Dialog] in 1.6: destroying dialog in `onHide` causes error
Reported by: | Kenneth G. Franqueiro | Owned by: | Kenneth G. Franqueiro |
---|---|---|---|
Priority: | high | Milestone: | 1.7 |
Component: | Dijit | Version: | 1.6.0 |
Keywords: | dialog | Cc: | |
Blocked By: | Blocking: |
Description
In Dojo 1.6, dijit.Dialog
's onHide
event is triggered when the hide animation ends. This is generally awesome.
However, the current implementation causes an error in one of the more frequent use-cases I've seen: hooking destruction of the dialog to this event. It throws because the code path that executes onHide
expects the _fadeOutDeferred
instance member to still be set after onHide
completes - however, when the widget is destroyed, the deferred is cancelled, and its canceller function deletes this instance member.
I've attached a patch that simply checks for existence of the member first.
For those who need to work around this in 1.6, it can be done simply by using setTimeout
to defer destruction until after the offending code path has completed:
dlg.connect(dlg, 'onHide', function() { setTimeout(function() { dlg.destroyRecursive(); }, 0); });
Attachments (3)
Change History (7)
Changed 10 years ago by
Attachment: | Dialog.diff added |
---|
Changed 10 years ago by
Attachment: | test12436.html added |
---|
comment:1 Changed 10 years ago by
Owner: | changed from bill to Kenneth G. Franqueiro |
---|
Sure, I'm fine w/it if you add an automated test (not using robot, just close the dialog using hide() or something).
Note that I have it on my list to get rid of the JS animations in widgets (replacing them with CSS animations), not sure when that will be done (hopefully in 1.7) but that will change the landscape some. Still good to have the test checked in though.
Changed 10 years ago by
Attachment: | Dialog.2.diff added |
---|
Revised change, no need for condition; hook on Deferred resolution
comment:2 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:3 Changed 10 years ago by
Milestone: | tbd → 1.7 |
---|
Simple isolated test case