Opened 11 years ago

Closed 10 years ago

#9180 closed defect (fixed)

1.3 dijit.Dialog / dijit._underlay uses references to self-created DialogUnderlay

Reported by: Phil DeJarnett Owned by: bill
Priority: low Milestone: 1.4
Component: Dijit Version: 1.3.0
Keywords: Cc:
Blocked By: Blocking:

Description

This is hard to explain, but in Dialog.js, if dijit._underlay is created by a specific dialog, that Underlay is always used for the fadeIn, due to the fact that it is assigned to the variable underlay. However, the fadeOut correctly refers to dijit._underlay directly.

This is an edge case, but the possibility exists that dijit._underlay was changed, and the Dialog now no longer refers to the correct underlay if it is shown at a later time.

I'm attaching a diff that replaces the underlay variable with dijit._underlay.

Note: this would be irrelevant if dijit.Dialog supported multiple Dialogs. The new single-underlay, while more efficient, makes this extremely difficult to pull off in custom code.

Attachments (1)

Dialog.patch (405 bytes) - added by Phil DeJarnett 11 years ago.
Diff that replaces "underlay" with "dijit._underlay"

Download all attachments as: .zip

Change History (7)

Changed 11 years ago by Phil DeJarnett

Attachment: Dialog.patch added

Diff that replaces "underlay" with "dijit._underlay"

comment:1 Changed 11 years ago by Adam Peller

Component: GeneralDijit
Owner: anonymous deleted

comment:2 Changed 11 years ago by bill

Milestone: tbd1.4
Owner: set to bill
Status: newassigned

This ticket must be related to your customization of Dialog, because dijit never destroys and recreates dijit._underlay.

Also, your diff file isn't parseable for some reason, probably because it doesn't list the file name.

In any case though, I'll change that code to defer creating the underlay until it's needed (ie, when the Dialog is shown), which will work around this problem.

comment:3 Changed 11 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [17375]) Defer creating underlay until it's needed. Fixes #9180 !strict.

comment:4 Changed 11 years ago by Phil DeJarnett

I didn't mean destroy, I was trying to set up multiple underlays (like I did before). I could have made this work without effort, except the fact that the first dijit.Dialog was referencing whatever underlay was created for all time, and therefore could be showing the "wrong" underlay.

It's only a problem when trying to set up multiple underlays. At any rate, your change ends up fixing the problem, because "var underlay" is now contained within a function.

Thanks!

(PS: I have no idea how to create patches, I should have labeled as a .diff)

comment:5 Changed 10 years ago by bill

Resolution: fixed
Status: closedreopened

[17375] creates a race condition that makes Dialog_a11y.html fail on IE6.

The problem happens in show(). show() sets up an onresize handler that calls layout(), and layout() references dijit._underlay. That onresize handler can be called before this._fadeIn.beforeBegin().

comment:6 Changed 10 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [18871]) Avoid race condition during show() when dijit._underlay may not yet be defined when onresize handler is called, fixes #9180 !strict.

Note: See TracTickets for help on using tickets.