jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/350789 )
Change subject: WindowManager: Add WindowInstance - a Promise-based lifecycle
object
......................................................................
WindowManager: Add WindowInstance - a Promise-based lifecycle object
This is in preparation for deprecating the nested opening-opened-closing Promise
returned by openWindow().
* Add WindowInstance class.
* Remove private 'opened' and 'opening' properties from WindowManager.
(No longer used.)
Update the one use of it outside WindowManger, in ProcessDialog.
* Improve documentation for the now-deprecated nested-Promise behaviour.
* Add compat code for calling .then() on openWindow().
This actually makes the currently broken OO.ui.alert() code work again
due to not being subject to the jQuery 3 unwrapping of nested promises.
Bug: T95923
Bug: T166153
Bug: T163510
Change-Id: I9dbc9852434caba15b797266289af532721f1f32
---
M build/modules.yaml
M jsduck.categories.json
M package.json
A src/WindowInstance.js
M src/WindowManager.js
M src/dialogs/ProcessDialog.js
6 files changed, 223 insertions(+), 118 deletions(-)
Approvals:
Bartosz Dziewoński: Looks good to me, approved
jenkins-bot: Verified
diff --git a/build/modules.yaml b/build/modules.yaml
index baeb1ef..d1e38d7 100644
--- a/build/modules.yaml
+++ b/build/modules.yaml
@@ -150,6 +150,7 @@
"src/ActionSet.js",
"src/Error.js",
"src/Process.js",
+ "src/WindowInstance.js",
"src/WindowManager.js",
"src/Window.js",
"src/Dialog.js",
diff --git a/jsduck.categories.json b/jsduck.categories.json
index 7ede227..07fbd39 100644
--- a/jsduck.categories.json
+++ b/jsduck.categories.json
@@ -11,6 +11,7 @@
"OO.ui.Toolbar",
"OO.ui.Window",
"OO.ui.Dialog",
+ "OO.ui.WindowInstance",
"OO.ui.WindowManager",
"OO.ui.Process",
"OO.ui.Error",
diff --git a/package.json b/package.json
index 2b7448d..720c282 100644
--- a/package.json
+++ b/package.json
@@ -17,6 +17,7 @@
"predoc": "grunt build",
"doc": "jsduck",
"postdoc": "grunt copy:jsduck",
+ "quickdoc": "grunt quick-build && jsduck && grunt copy:jsduck",
"publish-build": "grunt publish-build",
"demos": "grunt publish-build && grunt demos",
"jenkins": "npm test && jsduck && npm run postdoc"
diff --git a/src/WindowInstance.js b/src/WindowInstance.js
new file mode 100644
index 0000000..a423f21
--- /dev/null
+++ b/src/WindowInstance.js
@@ -0,0 +1,60 @@
+/**
+ * A window instance represents the life cycle for one single opening of a
window
+ * until its closing.
+ *
+ * While OO.ui.WindowManager will reuse OO.ui.Window objects, each time a
window is
+ * opened, a new lifecycle starts.
+ *
+ * For more information, please see the [OOjs UI documentation on MediaWiki]
[1].
+ *
+ * [1]: https://www.mediawiki.org/wiki/OOjs_UI/Windows
+ *
+ * @class
+ *
+ * @constructor
+ */
+OO.ui.WindowInstance = function OOuiWindowInstance() {
+ var state = {
+ opening: $.Deferred(),
+ opened: $.Deferred(),
+ closing: $.Deferred(),
+ closed: $.Deferred()
+ };
+
+ /**
+ * @private
+ * @property {Object}
+ */
+ this.state = state;
+
+ // Set these up as chained promises so that rejecting of
+ // an earlier stage automatically rejects the subsequent
+ // would-be stages as well.
+
+ /**
+ * @property {jQuery.Promise}
+ */
+ this.opening = state.opening.promise();
+ /**
+ * @property {jQuery.Promise}
+ */
+ this.opened = this.opening.then( function () {
+ return state.opened;
+ } );
+ /**
+ * @property {jQuery.Promise}
+ */
+ this.closing = this.opened.then( function () {
+ return state.closing;
+ } );
+ /**
+ * @property {jQuery.Promise}
+ */
+ this.closed = this.closing.then( function () {
+ return state.closed;
+ } );
+};
+
+/* Setup */
+
+OO.initClass( OO.ui.WindowInstance );
diff --git a/src/WindowManager.js b/src/WindowManager.js
index 34abfc0..1cc73ef 100644
--- a/src/WindowManager.js
+++ b/src/WindowManager.js
@@ -12,13 +12,11 @@
* {@link OO.ui.Window#open open} method is used, and the window manager
begins to open the window.
*
* - an `opening` event is emitted with an `opening` promise
- * - the #getSetupDelay method is called and the returned value is used to
time a pause in execution before
- * the window’s {@link OO.ui.Window#getSetupProcess getSetupProcess} method
is called on the
- * window and its result executed
+ * - the #getSetupDelay method is called and the returned value is used to
time a pause in execution before the
+ * window’s {@link OO.ui.Window#method-setup setup} method is called which
executes OO.ui.Window#getSetupProcess.
* - a `setup` progress notification is emitted from the `opening` promise
- * - the #getReadyDelay method is called the returned value is used to time a
pause in execution before
- * the window’s {@link OO.ui.Window#getReadyProcess getReadyProcess} method
is called on the
- * window and its result executed
+ * - the #getReadyDelay method is called the returned value is used to time a
pause in execution before the
+ * window’s {@link OO.ui.Window#method-ready ready} method is called which
executes OO.ui.Window#getReadyProcess.
* - a `ready` progress notification is emitted from the `opening` promise
* - the `opening` promise is resolved with an `opened` promise
*
@@ -68,9 +66,9 @@
this.factory = config.factory;
this.modal = config.modal === undefined || !!config.modal;
this.windows = {};
- this.opening = null;
- this.opened = null;
- this.closing = null;
+ // Deprecated placeholder promise given to compatOpening in openWindow()
+ // that is resolved in closeWindow().
+ this.compatOpened = null;
this.preparingToOpen = null;
this.preparingToClose = null;
this.currentWindow = null;
@@ -99,9 +97,9 @@
*
* @event opening
* @param {OO.ui.Window} win Window that's being opened
- * @param {jQuery.Promise} opening An `opening` promise resolved with a value
when the window is opened successfully.
- * When the `opening` promise is resolved, the first argument of the value is
an 'opened' promise, the second argument
- * is the opening data. The `opening` promise emits `setup` and `ready`
notifications when those processes are complete.
+ * @param {jQuery.Promise} opened A promise resolved with a value when the
window is opened successfully.
+ * This promise also emits `setup` and `ready` notifications. When this
promise is resolved, the first
+ * argument of the value is an 'closed' promise, the second argument is the
opening data.
* @param {Object} data Window opening data
*/
@@ -110,10 +108,9 @@
*
* @event closing
* @param {OO.ui.Window} win Window that's being closed
- * @param {jQuery.Promise} closing A `closing` promise is resolved with a
value when the window
- * is closed successfully. The promise emits `hold` and `teardown`
notifications when those
- * processes are complete. When the `closing` promise is resolved, the first
argument of its value
- * is the closing data.
+ * @param {jQuery.Promise} closed A promise resolved with a value when the
window is closed successfully.
+ * This promise also emits `hold` and `teardown` notifications. When this
promise is resolved, the first
+ * argument of its value is the closing data.
* @param {Object} data Window closing data
*/
@@ -196,7 +193,8 @@
* @return {boolean} Window is opening
*/
OO.ui.WindowManager.prototype.isOpening = function ( win ) {
- return win === this.currentWindow && !!this.opening &&
this.opening.state() === 'pending';
+ return win === this.currentWindow && !!this.lifecycle &&
+ this.lifecycle.opened.state() === 'pending';
};
/**
@@ -206,7 +204,9 @@
* @return {boolean} Window is closing
*/
OO.ui.WindowManager.prototype.isClosing = function ( win ) {
- return win === this.currentWindow && !!this.closing &&
this.closing.state() === 'pending';
+ return win === this.currentWindow && !!this.lifecycle &&
+ this.lifecycle.closing.state() === 'ready' &&
+ this.lifecycle.closed.state() === 'pending';
};
/**
@@ -216,7 +216,9 @@
* @return {boolean} Window is opened
*/
OO.ui.WindowManager.prototype.isOpened = function ( win ) {
- return win === this.currentWindow && !!this.opened &&
this.opened.state() === 'pending';
+ return win === this.currentWindow && !!this.lifecycle &&
+ this.lifecycle.opened.state() === 'ready' &&
+ this.lifecycle.closing.state() === 'pending';
};
/**
@@ -338,76 +340,98 @@
* @param {Object} [data] Window opening data
* @param {jQuery|null} [data.$returnFocusTo] Element to which the window will
return focus when closed.
* Defaults the current activeElement. If set to null, focus isn't changed on
close.
- * @return {jQuery.Promise} An `opening` promise resolved when the window is
done opening.
- * See {@link #event-opening 'opening' event} for more information about
`opening` promises.
+ * @return {OO.ui.WindowInstance|jQuery.Promise} A lifecycle object
representing this particular
+ * opening of the window. For backwards-compatibility, then object is also a
Thenable that is resolved
+ * when the window is done opening, with nested promise for when closing
starts. This behaviour
+ * is deprecated and is not compatible with jQuery 3 (T163510).
* @fires opening
*/
-OO.ui.WindowManager.prototype.openWindow = function ( win, data ) {
- var manager = this,
- opening = $.Deferred();
+OO.ui.WindowManager.prototype.openWindow = function ( win, data, lifecycle,
compatOpening ) {
+ var manager = this;
data = data || {};
+
+ // Internal parameter 'lifecycle' allows this method to always return
+ // a lifecycle even if the window still needs to be created
+ // asynchronously when 'win' is a string.
+ lifecycle = lifecycle || new OO.ui.WindowInstance();
+ compatOpening = compatOpening || $.Deferred();
+
+ // Turn lifecycle into a Thenable for backwards-compatibility with
+ // the deprecated nested-promise behaviour (T163510).
+ lifecycle.then = compatOpening.then;
// Argument handling
if ( typeof win === 'string' ) {
- return this.getWindow( win ).then( function ( win ) {
- return manager.openWindow( win, data );
- } );
+ this.getWindow( win ).then(
+ function ( win ) {
+ manager.openWindow( win, data, lifecycle,
compatOpening );
+ },
+ function ( err ) {
+ lifecycle.opening.reject( err );
+ }
+ );
+ return lifecycle;
}
// Error handling
if ( !this.hasWindow( win ) ) {
- opening.reject( new OO.ui.Error(
+ compatOpening.reject( new OO.ui.Error(
'Cannot open window: window is not attached to manager'
) );
- } else if ( this.preparingToOpen || this.opening || this.opened ) {
- opening.reject( new OO.ui.Error(
+ lifecycle.state.opening.reject( new OO.ui.Error(
+ 'Cannot open window: window is not attached to manager'
+ ) );
+ return lifecycle;
+ }
+ if ( this.preparingToOpen || ( this.lifecycle &&
this.lifecycle.closing.state() !== 'ready' ) ) {
+ compatOpening.reject( new OO.ui.Error(
'Cannot open window: another window is opening or open'
) );
+ lifecycle.state.opening.reject( new OO.ui.Error(
+ 'Cannot open window: another window is opening or open'
+ ) );
+ return lifecycle;
}
- // Window opening
- if ( opening.state() !== 'rejected' ) {
- // If a window is currently closing, wait for it to complete
- this.preparingToOpen = $.when( this.closing );
- // Ensure handlers get called after preparingToOpen is set
- this.preparingToOpen.done( function () {
- if ( manager.modal ) {
- manager.toggleGlobalEvents( true );
- manager.toggleAriaIsolation( true );
- }
- manager.$returnFocusTo = data.$returnFocusTo !==
undefined ? data.$returnFocusTo : $( document.activeElement );
- manager.currentWindow = win;
- manager.opening = opening;
- manager.preparingToOpen = null;
- manager.emit( 'opening', win, opening, data );
- setTimeout( function () {
- win.setup( data ).then( function () {
- manager.updateWindowSize( win );
- manager.opening.notify( { state:
'setup' } );
- setTimeout( function () {
- win.ready( data ).then(
function () {
- manager.opening.notify(
{ state: 'ready' } );
- manager.opening = null;
- manager.opened =
$.Deferred();
- opening.resolve(
manager.opened.promise(), data );
- }, function () {
- manager.opening = null;
- manager.opened =
$.Deferred();
- opening.reject();
- manager.closeWindow(
win );
- } );
- }, manager.getReadyDelay() );
- }, function () {
- manager.opening = null;
- manager.opened = $.Deferred();
- opening.reject();
- manager.closeWindow( win );
- } );
- }, manager.getSetupDelay() );
- } );
- }
+ // If a window is currently closing, wait for it to complete
+ this.preparingToOpen = $.when( this.lifecycle && this.lifecycle.closed
);
+ // Ensure handlers get called after preparingToOpen is set
+ this.preparingToOpen.done( function () {
+ if ( manager.modal ) {
+ manager.toggleGlobalEvents( true );
+ manager.toggleAriaIsolation( true );
+ }
+ manager.$returnFocusTo = data.$returnFocusTo !== undefined ?
data.$returnFocusTo : $( document.activeElement );
+ manager.currentWindow = win;
+ manager.lifecycle = lifecycle;
+ manager.preparingToOpen = null;
+ manager.emit( 'opening', win, compatOpening, data );
+ lifecycle.state.opening.resolve( data );
+ setTimeout( function () {
+ manager.compatOpened = $.Deferred();
+ win.setup( data ).then( function () {
+ manager.updateWindowSize( win );
+ compatOpening.notify( { state: 'setup' } );
+ setTimeout( function () {
+ win.ready( data ).then( function () {
+ compatOpening.notify( { state:
'ready' } );
+ lifecycle.state.opened.resolve(
data );
+ compatOpening.resolve(
manager.compatOpened.promise(), data );
+ }, function () {
+ lifecycle.state.opened.reject();
+ compatOpening.reject();
+ manager.closeWindow( win );
+ } );
+ }, manager.getReadyDelay() );
+ }, function () {
+ lifecycle.state.opened.reject();
+ compatOpening.reject();
+ manager.closeWindow( win );
+ } );
+ }, manager.getSetupDelay() );
+ } );
- return opening.promise();
+ return lifecycle;
};
/**
@@ -415,15 +439,16 @@
*
* @param {OO.ui.Window|string} win Window object or symbolic name of window
to close
* @param {Object} [data] Window closing data
- * @return {jQuery.Promise} A `closing` promise resolved when the window is
done closing.
- * See {@link #event-closing 'closing' event} for more information about
closing promises.
- * @throws {Error} An error is thrown if the window is not managed by the
window manager.
+ * @return {OO.ui.WindowInstance|jQuery.Promise} A lifecycle object
representing this particular
+ * opening of the window. For backwards-compatibility, the object is also a
Thenable that is resolved
+ * when the window is done closing (T163510).
* @fires closing
*/
OO.ui.WindowManager.prototype.closeWindow = function ( win, data ) {
var manager = this,
- closing = $.Deferred(),
- opened;
+ compatClosing = $.Deferred(),
+ lifecycle = this.lifecycle,
+ compatOpened;
// Argument handling
if ( typeof win === 'string' ) {
@@ -432,57 +457,73 @@
win = null;
}
+ // Turn lifecycle into a Thenable for backwards-compatibility with
+ // the deprecated nested-promise behaviour (T163510).
+ lifecycle.then = compatClosing.then;
+
// Error handling
if ( !win ) {
- closing.reject( new OO.ui.Error(
+ compatClosing.reject( new OO.ui.Error(
'Cannot close window: window is not attached to manager'
) );
- } else if ( win !== this.currentWindow ) {
- closing.reject( new OO.ui.Error(
+ lifecycle.state.closing.reject( new OO.ui.Error(
+ 'Cannot close window: window is not attached to manager'
+ ) );
+ return lifecycle;
+ }
+ if ( win !== this.currentWindow ) {
+ compatClosing.reject( new OO.ui.Error(
'Cannot close window: window already closed with
different data'
) );
- } else if ( this.preparingToClose || this.closing ) {
- closing.reject( new OO.ui.Error(
+ lifecycle.state.closing.reject( new OO.ui.Error(
+ 'Cannot close window: window already closed with
different data'
+ ) );
+ return lifecycle;
+ }
+ if ( this.preparingToClose || this.closing ) {
+ compatClosing.reject( new OO.ui.Error(
'Cannot close window: window already closing with
different data'
) );
+ lifecycle.state.closing.reject( new OO.ui.Error(
+ 'Cannot close window: window already closing with
different data'
+ ) );
+ return lifecycle;
}
- // Window closing
- if ( closing.state() !== 'rejected' ) {
- // If the window is currently opening, close it when it's done
- this.preparingToClose = $.when( this.opening );
- // Ensure handlers get called after preparingToClose is set
- this.preparingToClose.always( function () {
- manager.closing = closing;
- manager.preparingToClose = null;
- manager.emit( 'closing', win, closing, data );
- opened = manager.opened;
- manager.opened = null;
- opened.resolve( closing.promise(), data );
- setTimeout( function () {
- win.hold( data ).then( function () {
- closing.notify( { state: 'hold' } );
- setTimeout( function () {
- win.teardown( data ).then(
function () {
- closing.notify( {
state: 'teardown' } );
- if ( manager.modal ) {
-
manager.toggleGlobalEvents( false );
-
manager.toggleAriaIsolation( false );
- }
- if (
manager.$returnFocusTo && manager.$returnFocusTo.length ) {
-
manager.$returnFocusTo[ 0 ].focus();
- }
- manager.closing = null;
- manager.currentWindow =
null;
- closing.resolve( data );
- } );
- }, manager.getTeardownDelay() );
- } );
- }, manager.getHoldDelay() );
- } );
- }
+ // If the window is currently opening, close it when it's done
+ this.preparingToClose = $.when( this.lifecycle.opened );
+ // Ensure handlers get called after preparingToClose is set
+ this.preparingToClose.always( function () {
+ manager.preparingToClose = null;
+ manager.emit( 'closing', win, compatClosing, data );
+ lifecycle.state.closing.resolve( data );
+ compatOpened = manager.compatOpened;
+ manager.compatOpened = null;
+ compatOpened.resolve( compatClosing.promise(), data );
+ setTimeout( function () {
+ win.hold( data ).then( function () {
+ compatClosing.notify( { state: 'hold' } );
+ setTimeout( function () {
+ win.teardown( data ).then( function () {
+ compatClosing.notify( { state:
'teardown' } );
+ if ( manager.modal ) {
+
manager.toggleGlobalEvents( false );
+
manager.toggleAriaIsolation( false );
+ }
+ if ( manager.$returnFocusTo &&
manager.$returnFocusTo.length ) {
+ manager.$returnFocusTo[
0 ].focus();
+ }
+ manager.currentWindow = null;
+ manager.lifecycle = null;
+ lifecycle.state.closed.resolve(
data );
+ compatClosing.resolve( data );
+ } );
+ }, manager.getTeardownDelay() );
+ } );
+ }, manager.getHoldDelay() );
+ } );
- return closing.promise();
+ return lifecycle;
};
/**
diff --git a/src/dialogs/ProcessDialog.js b/src/dialogs/ProcessDialog.js
index 65ddd75..e5b853e 100644
--- a/src/dialogs/ProcessDialog.js
+++ b/src/dialogs/ProcessDialog.js
@@ -239,7 +239,8 @@
} else if ( this.isOpening() ) {
if ( !this.fitOnOpen ) {
// Size is relative and the dialog isn't open
yet, so wait.
- this.manager.opening.done( this.fitLabel.bind(
this ) );
+ // FIXME: This should ideally be handled by
setup somehow.
+ this.manager.lifecycle.opened.done(
this.fitLabel.bind( this ) );
this.fitOnOpen = true;
}
return;
--
To view, visit https://gerrit.wikimedia.org/r/350789
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9dbc9852434caba15b797266289af532721f1f32
Gerrit-PatchSet: 22
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: Divec <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Trevor Parscal <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
