Page MenuHomePhabricator

WindowInstance private deferreds and public promises can be inconsistent due to jQuery 3 asynchronous promise resolution
Closed, ResolvedPublic

Description

The state of WindowInstance private deferreds (e.g. lifecycle.deferreds.closing) and public promises (e.g. lifecycle.closing) can be inconsistent due to jQuery 3 asynchronous promise resolution. The public promise will lag behind the private deferred by one "tick".

Our code does not correctly account for this. It should not cause any problems when simply waiting for promises to resolve, but we check their .state() in a few places and that causes issues.

In particular, some examples:

  • (T169844) Trying to close a window that is already closing while the states of closing promise are inconsistent causes exceptions. Minimal test case: open "Quick alert" on https://doc.wikimedia.org/oojs-ui/master/demos/?page=dialogs, then run this in the console:

    [Note, this isn't actually visible in the demo right now since we just merged a workaround for that bug. Use a local build without that patch.]
manager = OO.ui.getWindowManager();
win = manager.getCurrentWindow();
setTimeout( function () { OO.ui.getWindowManager().closeWindow( win ); } ); // "Uncaught TypeError: Cannot read property 'resolve' of null"
OO.ui.getWindowManager().closeWindow( win );
manager = OO.ui.getWindowManager();
win = manager.getCurrentWindow();
manager.closeWindow( win );
console.log( manager.isClosing( win ) ); // should log true, actually logs false

Event Timeline

Change 364259 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[oojs/ui@master] WindowManager: Avoid inconsistent state due to asynchronous promise resolution

https://gerrit.wikimedia.org/r/364259

Change 364259 merged by jenkins-bot:
[oojs/ui@master] WindowManager: Avoid inconsistent state due to asynchronous promise resolution

https://gerrit.wikimedia.org/r/364259

Krinkle triaged this task as Medium priority.
Krinkle removed a project: Patch-For-Review.