Opened 10 years ago

Closed 9 years ago

#10453 closed defect (fixed)

Dialog: uninitialize may cause problem on nested dialog (race condition with fadeOut)

Reported by: Manuel Irrschik Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.4.0b
Keywords: dijit.Dialog nested underlay Cc:
Blocked By: Blocking:

Description

i've noticed a (minor) problem with nested Dialogs:

if the upper most dialog's unitialize method is called before the fadeOut is complete (e.g. dialog is destroyed or so) the underlay is hidden directly instead of running through the new nested-dialog code.

(see Line 252 of Dialog.js http://bugs.dojotoolkit.org/browser/dijit/trunk/Dialog.js?rev=#L252)

fix should be to move the nested-dialog code to a separate function and call it from both places (i appended a possible fix which i am using currently)

Attachments (1)

Dialog.patch (835 bytes) - added by Manuel Irrschik 10 years ago.

Download all attachments as: .zip

Change History (7)

Changed 10 years ago by Manuel Irrschik

Attachment: Dialog.patch added

comment:1 Changed 10 years ago by bill

Milestone: 1.41.5
Summary: dijit.Dialog: uninitialize may cause problem on nested dialog (race condition with fadeOut)Dialog: uninitialize may cause problem on nested dialog (race condition with fadeOut)

OK, thanks, I'll take a look for 1.5. Can you sign a CLA (http://www.dojofoundation.org/cla) so that we can look at your patch?

There are some other tickets open about Dialog related to race conditions too.

comment:2 in reply to:  1 Changed 10 years ago by Manuel Irrschik

Replying to bill:

Can you sign a CLA (http://www.dojofoundation.org/cla) so that we can look at your patch?

sry, i wasn't aware of that, just signed it and sent it by mail

comment:3 Changed 10 years ago by bill

Thanks! (And future patches always appreciated too.)

comment:4 Changed 9 years ago by Adam Peller

Milestone: 1.51.6

comment:5 Changed 9 years ago by bill

Owner: set to bill
Status: newassigned

This was fixed in [22662], although I did just notice that the destroy() (aka uninitialize()) code should be cancelling any running animations, so I'll check that in.

comment:6 Changed 9 years ago by bill

Resolution: fixed
Status: assignedclosed

Oh actually, it already does that. So this is fixed already. (But please double check on your test case.)

Note: See TracTickets for help on using tickets.