jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/364259 )
Change subject: WindowManager: Avoid inconsistent state due to asynchronous
promise resolution
......................................................................
WindowManager: Avoid inconsistent state due to asynchronous promise resolution
WindowInstance:
* New methods #isOpening, #isOpened, #isClosing, #isClosed.
These should be used instead of checking the .state() of
the promises #opening, #opened, #closing, #closed, as that
is updated asynchronously and not immediately when the
window lifecycle stage changes.
WindowManager:
* Use the new WindowInstance methods instead of checking .state().
* Tweak error handling in #openWindow to reduce code duplication.
Bug: T170164
Change-Id: I63b81e12a9ffdf8b95fa7cccfa984672d534b4ba
---
M src/WindowInstance.js
M src/WindowManager.js
2 files changed, 54 insertions(+), 22 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/WindowInstance.js b/src/WindowInstance.js
index 72490c7..0e900be 100644
--- a/src/WindowInstance.js
+++ b/src/WindowInstance.js
@@ -58,3 +58,41 @@
/* Setup */
OO.initClass( OO.ui.WindowInstance );
+
+/**
+ * Check if window is opening.
+ *
+ * @return {boolean} Window is opening
+ */
+OO.ui.WindowInstance.prototype.isOpening = function () {
+ return this.deferreds.opened.state() === 'pending';
+};
+
+/**
+ * Check if window is opened.
+ *
+ * @return {boolean} Window is opened
+ */
+OO.ui.WindowInstance.prototype.isOpened = function () {
+ return this.deferreds.opened.state() === 'resolved' &&
+ this.deferreds.closing.state() === 'pending';
+};
+
+/**
+ * Check if window is closing.
+ *
+ * @return {boolean} Window is closing
+ */
+OO.ui.WindowInstance.prototype.isClosing = function () {
+ return this.deferreds.closing.state() === 'resolved' &&
+ this.deferreds.closed.state() === 'pending';
+};
+
+/**
+ * Check if window is closed.
+ *
+ * @return {boolean} Window is closed
+ */
+OO.ui.WindowInstance.prototype.isClosed = function () {
+ return this.deferreds.closed.state() === 'resolved';
+};
diff --git a/src/WindowManager.js b/src/WindowManager.js
index 2c58fe4..1a256d3 100644
--- a/src/WindowManager.js
+++ b/src/WindowManager.js
@@ -194,7 +194,7 @@
*/
OO.ui.WindowManager.prototype.isOpening = function ( win ) {
return win === this.currentWindow && !!this.lifecycle &&
- this.lifecycle.opened.state() === 'pending';
+ this.lifecycle.isOpening();
};
/**
@@ -205,8 +205,7 @@
*/
OO.ui.WindowManager.prototype.isClosing = function ( win ) {
return win === this.currentWindow && !!this.lifecycle &&
- this.lifecycle.closing.state() === 'resolved' &&
- this.lifecycle.closed.state() === 'pending';
+ this.lifecycle.isClosing();
};
/**
@@ -217,8 +216,7 @@
*/
OO.ui.WindowManager.prototype.isOpened = function ( win ) {
return win === this.currentWindow && !!this.lifecycle &&
- this.lifecycle.opened.state() === 'resolved' &&
- this.lifecycle.closing.state() === 'pending';
+ this.lifecycle.isOpened();
};
/**
@@ -347,7 +345,8 @@
* @fires opening
*/
OO.ui.WindowManager.prototype.openWindow = function ( win, data, lifecycle,
compatOpening ) {
- var manager = this;
+ var error,
+ manager = this;
data = data || {};
// Internal parameter 'lifecycle' allows this method to always return
@@ -384,21 +383,16 @@
// Error handling
if ( !this.hasWindow( win ) ) {
- compatOpening.reject( new OO.ui.Error(
- 'Cannot open window: window is not attached to manager'
- ) );
- lifecycle.deferreds.opening.reject( new OO.ui.Error(
- 'Cannot open window: window is not attached to manager'
- ) );
- return lifecycle;
+ error = 'Cannot open window: window is not attached to manager';
+ } else if ( this.lifecycle && this.lifecycle.isOpened() ) {
+ error = 'Cannot open window: another window is open';
+ } else if ( this.preparingToOpen || ( this.lifecycle &&
this.lifecycle.isOpening() ) ) {
+ error = 'Cannot open window: another window is opening';
}
- if ( this.preparingToOpen || ( this.lifecycle &&
this.lifecycle.closing.state() !== 'resolved' ) ) {
- compatOpening.reject( new OO.ui.Error(
- 'Cannot open window: another window is opening or open'
- ) );
- lifecycle.deferreds.opening.reject( new OO.ui.Error(
- 'Cannot open window: another window is opening or open'
- ) );
+
+ if ( error ) {
+ compatOpening.reject( new OO.ui.Error( error ) );
+ lifecycle.deferreds.opening.reject( new OO.ui.Error( error ) );
return lifecycle;
}
@@ -472,9 +466,9 @@
error = 'Cannot close window: no window is currently open';
} else if ( !win ) {
error = 'Cannot close window: window is not attached to
manager';
- } else if ( win !== this.currentWindow ) {
+ } else if ( win !== this.currentWindow || this.lifecycle.isClosed() ) {
error = 'Cannot close window: window already closed with
different data';
- } else if ( this.preparingToClose ||
lifecycle.deferreds.closing.state() === 'resolved' ) {
+ } else if ( this.preparingToClose || this.lifecycle.isClosing() ) {
error = 'Cannot close window: window already closing with
different data';
}
--
To view, visit https://gerrit.wikimedia.org/r/364259
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I63b81e12a9ffdf8b95fa7cccfa984672d534b4ba
Gerrit-PatchSet: 2
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <[email protected]>
Gerrit-Reviewer: DLynch <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
