Trevor Parscal has uploaded a new change for review.

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

Change subject: Close the save dialog on teardown only, not on save
......................................................................

Close the save dialog on teardown only, not on save

Closing a dialog with specific data means closing it again with potentially 
different data, while the dialog is already closing, means someone wins and 
someone looses. Silently failing in this case is bad, because if the first 
close call was a cancel, producing no side effects, but the second close call 
would have produced some side effect, the side effect would never occur.

The problem here really was that the save dialog needs to be closed before we 
can destroy the surface so we can uphold the assumption that hold and teardown 
processes are operating on an attached DOM.

The solution is to automatically close the save dialog on teardown, rather than 
on save. Since save triggers teardown, this has and identical user experience.

Change-Id: I669448739f168737d4eddd6496189a819ce894b1
---
M modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
1 file changed, 3 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor 
refs/changes/84/146984/1

diff --git a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js 
b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
index 44f457f..3598872 100644
--- a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
+++ b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
@@ -470,7 +470,6 @@
                        } );
                        this.revid = newid;
                }
-               this.saveDialog.close();
                this.saveDialog.reset();
                this.replacePageContent( html, categoriesHtml, isRedirect, 
displayTitle );
                this.setupSectionEditLinks();
@@ -1090,9 +1089,11 @@
        if ( this.surface.mwTocWidget ) {
                this.surface.mwTocWidget.teardown();
        }
+
        if ( this.saveDialog ) {
-               // If we got as far as setting up the save dialog, tear it down
+               // If the save dialog is still open (from saving) close it
                promises.push( this.saveDialog.close() );
+               // Release the reference
                this.saveDialog = null;
        }
 

-- 
To view, visit https://gerrit.wikimedia.org/r/146984
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I669448739f168737d4eddd6496189a819ce894b1
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Trevor Parscal <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to