Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/186100
Change subject: [WIP] Make sanity check synchronous
......................................................................
[WIP] Make sanity check synchronous
It's doubtful whether making it asynchronous is helpful,
because of the overhead involved in making a copy of the
linear model, and because it'll still freeze the editor,
just after we indicate to the user that it's ready instead
of before.
TODO: Figure out whether this is better or worse than
what we had before.
Bug: T87161
Change-Id: I3bcc4675b7bc388884fc17e38ff37d0cf1aae4ef
---
M modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css
M modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
M modules/ve-mw/init/ve.init.mw.Target.js
3 files changed, 26 insertions(+), 68 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/00/186100/1
diff --git a/modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css
b/modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css
index 3f71eaf..e14032a 100644
--- a/modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css
+++ b/modules/ve-mw/init/styles/ve.init.mw.ViewPageTarget.css
@@ -46,8 +46,3 @@
margin-right: 0.25em;
margin-top: 0.2em;
}
-
-/* Needs to override .oo-ui.widget.oo-ui-widget-disabled */
-.ve-init-mw-viewPageTarget-waiting.oo-ui-widget.oo-ui-widget-disabled {
- cursor: progress;
-}
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 cd1c625..e69056e 100644
--- a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
+++ b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
@@ -94,8 +94,7 @@
showChanges: 'onShowChanges',
showChangesError: 'onShowChangesError',
noChanges: 'onNoChanges',
- serializeError: 'onSerializeError',
- sanityCheckComplete: 'updateToolbarSaveButtonState'
+ serializeError: 'onSerializeError'
} );
if ( currentUri.query.venotify ) {
@@ -839,10 +838,9 @@
var isDisabled;
this.edited = this.getSurface().getModel().hasBeenModified();
- // Disable the save button if we have no history or if the sanity check
is not finished
- isDisabled = ( !this.edited && !this.restoring ) ||
!this.sanityCheckFinished;
+ // Disable the save button if we have no history
+ isDisabled = !this.edited && !this.restoring;
this.toolbarSaveButton.setDisabled( isDisabled );
- this.toolbarSaveButton.$element.toggleClass(
've-init-mw-viewPageTarget-waiting', !this.sanityCheckFinished );
mw.hook( 've.toolbarSaveButton.stateChanged' ).fire( isDisabled );
};
@@ -1011,7 +1009,7 @@
}
}
- if ( this.sanityCheckPromise.state() === 'rejected' ) {
+ if ( !this.sanityCheckVerified ) {
options.needcheck = 1;
}
diff --git a/modules/ve-mw/init/ve.init.mw.Target.js
b/modules/ve-mw/init/ve.init.mw.Target.js
index 2c2b6f4..4b692bc 100644
--- a/modules/ve-mw/init/ve.init.mw.Target.js
+++ b/modules/ve-mw/init/ve.init.mw.Target.js
@@ -39,11 +39,6 @@
.extend( { action: 'submit' } );
this.events = new ve.init.mw.TargetEvents( this );
- /**
- * @property {jQuery.Promise|null}
- */
- this.sanityCheckPromise = null;
-
this.modules = [
'ext.visualEditor.mwcore',
'ext.visualEditor.mwlink',
@@ -177,10 +172,6 @@
/**
* @event serializeComplete
* Fired when serialization is complete
- */
-
-/**
- * @event sanityCheckComplete
*/
/* Static Properties */
@@ -520,7 +511,7 @@
this.loading = false;
this.edited = false;
this.setupSurface( this.doc, function () {
- target.startSanityCheck();
+ target.sanityCheck();
target.emit( 'surfaceReady' );
} );
};
@@ -1059,7 +1050,6 @@
this.$checkboxes = null;
this.remoteNotices = [];
this.localNoticeMessages = [];
- this.sanityCheckFinished = false;
this.sanityCheckVerified = false;
};
@@ -1404,62 +1394,37 @@
};
/**
- * Fire off the sanity check. Must be called before the surface is activated.
+ * Perform the sanity check. Must be called before the surface is activated.
*
- * To access the result, check whether #sanityCheckPromise has been resolved
or rejected
- * (it's asynchronous, so it may still be pending when you check).
+ * To access the result, check #sanityCheckVerified
*
* @method
- * @fires sanityCheckComplete
*/
-ve.init.mw.Target.prototype.startSanityCheck = function () {
- // We have to get a copy of the data now, before we unlock the surface
and let the user edit,
- // but we can defer the actual conversion and comparison
- var target = this,
- doc = this.getSurface().getModel().getDocument(),
- data = new ve.dm.FlatLinearData( doc.getStore().clone(),
ve.copy( doc.getFullData() ) ),
+ve.init.mw.Target.prototype.sanityCheck = function () {
+ var i,
+ oldDoc = this.getSurface().getModel().getDocument(),
oldDom = this.doc,
- d = $.Deferred();
+ len = oldDom.body.childNodes.length,
+ data = new ve.dm.FlatLinearData( oldDoc.getStore(),
oldDoc.getFullData() ),
+ newDoc = new ve.dm.Document( data, oldDom, undefined,
oldDoc.getInternalList(), oldDoc.getInnerWhitespace(), oldDoc.getLang(),
oldDoc.getDir() ),
+ newDom = ve.dm.converter.getDomFromModel( newDoc );
- // Reset
- this.sanityCheckFinished = false;
- this.sanityCheckVerified = false;
+ // We can't compare oldDom.body and newDom.body directly, because the
attributes on the
+ // <body> were ignored in the conversion. So compare each child
separately.
- setTimeout( function () {
- // We can't compare oldDom.body and newDom.body directly,
because the attributes on the
- // <body> were ignored in the conversion. So compare each child
separately.
- var i,
- len = oldDom.body.childNodes.length,
- newDoc = new ve.dm.Document( data, oldDom, undefined,
doc.getInternalList(), doc.getInnerWhitespace(), doc.getLang(), doc.getDir() ),
- newDom = ve.dm.converter.getDomFromModel( newDoc );
-
- // Explicitly unlink our full copy of the original version of
the document data
- data = undefined;
-
- if ( len !== newDom.body.childNodes.length ) {
- // Different number of children, so they're definitely
different
- d.reject();
+ if ( len !== newDom.body.childNodes.length ) {
+ // Different number of children, so they're definitely different
+ this.sanityCheckVerified = false;
+ return;
+ }
+ for ( i = 0; i < len; i++ ) {
+ if ( !oldDom.body.childNodes[i].isEqualNode(
newDom.body.childNodes[i] ) ) {
+ this.sanityCheckVerified = false
return;
}
- for ( i = 0; i < len; i++ ) {
- if ( !oldDom.body.childNodes[i].isEqualNode(
newDom.body.childNodes[i] ) ) {
- d.reject();
- return;
- }
- }
- d.resolve();
- } );
+ }
- this.sanityCheckPromise = d.promise()
- .done( function () {
- // If we detect no roundtrip errors,
- // don't emphasize "review changes" to the user.
- target.sanityCheckVerified = true;
- })
- .always( function () {
- target.sanityCheckFinished = true;
- target.emit( 'sanityCheckComplete' );
- } );
+ this.sanityCheckVerified = true;
};
/**
--
To view, visit https://gerrit.wikimedia.org/r/186100
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3bcc4675b7bc388884fc17e38ff37d0cf1aae4ef
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
