Opened 7 years ago

Closed 7 years ago

Last modified 5 years ago

#16152 closed defect (fixed)

Dialog: calling show() while hide() in progress confuses DialogLevelManager --- Remaining overlay blocks page

Reported by: Paul Christopher Owned by: bill
Priority: undecided Milestone: 1.6.3
Component: Dijit Version: 1.8.0
Keywords: Cc:
Blocked By: Blocking:

Description

Description

Multiple calls to dialog.show and dialog.hide confuse the DialogLevelManager. The last call in the chain of dialog.hide does hide the dialog, however the underlay remains on the screen and makes the whole page not usable for the user anymore since the overlay prevents any interactions with the mouse.

Steps to reproduce the issue

  • In Dialog.js add some console.logs to the show/hide method of the DialogLevelManager. Especially dump the dialogstack ds each them. Do also add console.logs to _DialogBase.show/hide and check this.open there.
  • Run the attached test case

Discussion

The underlay is not closed because the dialogstack is full with new instances of the displayed dialog - although the dialog isn't displayed anymore. Don't know why this happens. "this.open" in _DialogBase.show seems not to be recognized (is "false")and each time dialog.show is called, a new dialog is opened?

This is a reduced real life example in which we used an Ajax loader based on dijit/Dialog. We changed the application code now to prevent those multiple calls. Nevertheless I wanted to share the test case here. Maybe this edge case also needs some corrections to Dialog.js?

Attachments (2)

testDialog.html (1.6 KB) - added by Paul Christopher 7 years ago.
PatchDialog1-8-0[CLA].diff (2.2 KB) - added by Paul Christopher 7 years ago.

Download all attachments as: .zip

Change History (13)

Changed 7 years ago by Paul Christopher

Attachment: testDialog.html added

comment:1 Changed 7 years ago by Paul Christopher

The problem seems to be the call of DialogLevelManager.hide(this) (dijit/Dialog.js line 432) inside the onEnd callback of the fx.fadeOut. If DialogLevelManager.hide(this) is called immediately after this._set("open", false) (line 449) (and not after the fad out has finished), there would be no problems with the given test case. On the other hand, this would change a lot: The underlay would be hidden before the dialog is hidden, and that's not really what we want...

comment:2 Changed 7 years ago by Paul Christopher

To fix the hide order issue of dialog and underlay: Maybe it is enough to give DialogUnderlay._singleton.hide(); (line 571) a setTimeout with manager.defaultDuration and if the dialog is shown again just a clearTimeout?

Last edited 7 years ago by Paul Christopher (previous) (diff)

Changed 7 years ago by Paul Christopher

Attachment: PatchDialog1-8-0[CLA].diff added

comment:3 Changed 7 years ago by Paul Christopher

Added a proposal for a patch but don't know whether this really fits into the architecture.

comment:4 Changed 7 years ago by bill

Milestone: tbd1.9
Status: newassigned
Summary: dijit/Dialog: Multiple show/hide calls confuse DialogLevelManager --- Remaining overlay blocks pageDialog: calling show() while hide() in progress confuses DialogLevelManager --- Remaining overlay blocks page

I see, thanks for the test case. We do have some tests in dijit/tests/Dialog.html, such as "concurrent hide show", where things work correctly, but obviously not in the case you showed.

As you said, the DialogLevelManager.hide(this) call comes at the end of the fade-out. And Dialog.show() cancels an in-progress Dialog.hide(). So when the Dialog.hide() is interrupted by a Dialog.show(), DialogLevelManager.hide(this) never gets called. But Dialog.show() still calls DialogLevelManager.show(this, this.underlayAttrs) again, hence the imbalance.

I think rather than calling DialogLevelManager.hide(this) early like in your patch, I'll modify Dialog.show() to act differently when it's cancelling an in-progress hide.

comment:5 Changed 7 years ago by bill

Resolution: fixed
Status: assignedclosed

In [29836]:

Fix bug calling Dialog.show() while hide() fade-out is in progress, causing Dialog to be added to DialogLevelManager stack twice. Fixes #16152 !strict.

comment:6 Changed 6 years ago by heyuser620

Team,

We are still on Dojo 1.6 and facing same issue, how can I get the fix for same issue for Dojo 1.6?

Thanks in advance.

Last edited 6 years ago by heyuser620 (previous) (diff)

comment:7 Changed 6 years ago by bill

Git commit: 86e4c01a08c57412a836112d1f50a2ef4f80abe3

comment:8 Changed 5 years ago by Bill Keese <bill@…>

In d23a4848a00e9abef8e105c64f1265399b84f20f/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:9 Changed 5 years ago by Bill Keese <bill@…>

In ee0f8e1cc150f4ba6947816d22dc0b9f194ca09b/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:10 Changed 5 years ago by Bill Keese <bill@…>

In 86e517a180f234b2f67388badaaa52b97b4f4262/dijit:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:11 Changed 5 years ago by bill

Milestone: 1.91.6.3
Note: See TracTickets for help on using tickets.