[CLOSED] brackets.showModalDialog() breaks if similar dialogs are ever stacked (shown simultaneously)

[core-ai-bot]

Issue by peterflynn Wednesday Feb 01, 2012 at 23:07 GMT Originally opened as https://github.com/adobe/brackets/issues/187

---

This is due to an underlying API bug/feature in Bootstrap: if you try to show a dialog while it is already open, the dialog is simply closed instead. (This is true for any pair of dialogs of the same "type," i.e. any two dialogs with the same id, i.e. any two dialogs that use the same underlying chunk of HTML).

Ideally, it would work fine to stack multiple dialogs atop each other (although ugly, there may be edge cases where this is necessary).

But since it's technically infeasible to instantiate the same "type" of dialog twice -- due to there being only one copy of the HTML it needs -- we should throw an exception if someone tries to show a second one. (Rather than doing something unexpected, silently closing the first dialog as if the user had canceled it even though they haven't).

[core-ai-bot]

Comment by peterflynn Wednesday Feb 01, 2012 at 23:10 GMT

---

Btw, the need for each type of dialog to have a permanent chunk of HTML in index.html is probably an architectural issue in itself, in the sense that it doesn't scale particularly well.

It's also not a great precedent as we add more UI like Find, preferences, etc. If index.html must contain every piece of UI our app will ever show -- with most of it set to display:none 90% of the time -- we'll wind up with an enormous file that's hard to maintain.

[core-ai-bot]

Comment by peterflynn Thursday Feb 02, 2012 at 00:30 GMT

---

A closely related issue: Bootstrap seems to have timing bugs related to showing/hiding the modal overlay if two dialogs are shown with too short a time delay between them. Sample code:

        brackets.showModalDialog(
            brackets.DIALOG_ID_SAVE_CLOSE,
            "test",
            "Test dialog 1"
        ).done(function (id) {

            setTimeout(function () {
                brackets.showModalDialog(
                    brackets.DIALOG_ID_SAVE_CLOSE,
                    "test",
                    "Test dialog 2"
                );
            }, 0);

            // brackets.showModalDialog(
            //     brackets.DIALOG_ID_SAVE_CLOSE,
            //     "test",
            //     "Test dialog 2"
            // );
        });

With the setTimeout(), the second dialog will never be displayed and, although the visible gray tint goes away, the UI remains "locked" and unusable -- no interaction is accepted anymore. Without a setTimeout(), the second dialog will be displayed, but neither overlay is ever removed, so after dismissing the second dialog the user is stuck on a solid black screen.

[core-ai-bot]

Comment by peterflynn Thursday Feb 02, 2012 at 00:32 GMT

---

Yep, in fact you can repro this in master simply by hitting two errors quickly. E.g. -- try to open a file that can't be opened for some reason. Dismiss the error dialog and very quickly try to select the same file again. UI will be left in an unusable state.

Bumping up priority since this can cause data loss, etc.

[core-ai-bot]

Comment by peterflynn Thursday Feb 02, 2012 at 00:34 GMT

---

Workaround: remove the "fade" CSS class from all our dialogs.

[core-ai-bot]

Comment by peterflynn Thursday Feb 02, 2012 at 00:39 GMT

---

Ok, the 'showing sequentially' bug actually appears fairly unrelated, and is much more severe and easier to repro (and has a much more obvious workaround)... so I'm forking that off into a separate bug, #191. This one can remain as a low-priority architectural issue.

[core-ai-bot]

Comment by jlondon Friday Feb 03, 2012 at 06:55 GMT

---

Recommending for Sprint 4 Bug Fix Story

[core-ai-bot]

Comment by njx Wednesday Feb 15, 2012 at 01:08 GMT

---

Peter said he tested this as part of reviewing the code. Closing.