adobe / brackets

An open source code editor for the web, written in JavaScript, HTML and CSS.
http://brackets.io
MIT License
33.25k stars 7.63k forks source link

UI left unusable if two dialogs are shown too close together in time #191

Closed peterflynn closed 12 years ago

peterflynn commented 12 years ago

Bootstrap seems to have timing bugs related to showing/hiding the UI-blocking modal overlay if two dialogs are shown with too short a time delay between them.

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. The UI will be left solid black, and entirely unusable. Can cause data loss, since now all unsaved changes are un-saveable.

Here's sample code to repro more reliably:

        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.

peterflynn commented 12 years ago

Workaround: remove the "fade" CSS class from all our dialogs AND ensure at least a setTimeout(..., 0) in between dialogs. (Without the setTimeout(), Bootstrap will crash when we try to show the 2nd dialog because some of its code that is trying to close the last dialog is still running -- even without the fade).

The setTimeout() requirement is kind of crappy since it means interposing an async wait between any code that might be triggered by a dialog dismissal event and any code that might show a dialog. It's hard for code to stay aware of everyone who might be calling it...

peterflynn commented 12 years ago

This is blocking the preferred UI design for the "external changes" work, so I'm assigning to me and setting it for Sprint 3.

peterflynn commented 12 years ago

I think it might be relatively simple to fix the setTimeout() crash in Bootstrap -- without fixing all the other issues around "fade." Then we could easily work around those remaining bugs simply by not using "fade"... with no hard-to-maintain JS changes being imposed.

peterflynn commented 12 years ago

Note: this was fixed by pull request #196 (the "referenced by..." link hasn't appeared here because the reference was in the pull request's title, which GitHub evidently isn't smart enough to spot).