Divec has uploaded a new change for review.
https://gerrit.wikimedia.org/r/102009
Change subject: WIP: Reduce CE-DM-CE echo when text is replaced
......................................................................
WIP: Reduce CE-DM-CE echo when text is replaced
Proof of concept: don't cause needless surface refreshes on overwriting.
WHAT DOES IT FIX?
* In the demo, select a letter in the middle of the text
* Press a letter key to overwrite it
* The 350ms grey flash no longer happens: ContentBranchNode isn't echoing
changes
demos/ve/index.php
* Temporary auto model dump (helpful for testing on tiny documents)
ve.ce.Surface
* Flag the connection between Insertion/Removal change pairs (it's really a
replacement)
* Disable handleInsertion. TODO: we're not ready to move away from
handleInsertion yet
ve.dm.Surface->Document->TransactionProcessor->DocumentSynchronizer->Node
* Pass the flag down to the point where an event should be emitted or not
* TODO: This is somewhat ugly: how deep must the flag go? Is there a better way?
ve.ce.ContentBranchNode
* Ignore changes which have no effect
* TODO: The test may need to be more/less careful
Change-Id: Ied06031e9f25ea6ac80c64035f04eef82ce46df0
---
M demos/ve/index.php
M modules/ve/ce/ve.ce.ContentBranchNode.js
M modules/ve/ce/ve.ce.Surface.js
M modules/ve/dm/ve.dm.Document.js
M modules/ve/dm/ve.dm.DocumentSynchronizer.js
M modules/ve/dm/ve.dm.Node.js
M modules/ve/dm/ve.dm.Surface.js
M modules/ve/dm/ve.dm.TransactionProcessor.js
8 files changed, 66 insertions(+), 30 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/09/102009/1
diff --git a/demos/ve/index.php b/demos/ve/index.php
index 8bf2983..06473d9 100644
--- a/demos/ve/index.php
+++ b/demos/ve/index.php
@@ -347,7 +347,7 @@
// TODO: Validate input
console.dir(
ve.instances[0].view.documentView.model.data.slice( start, end ) );
} );
- dumpModelButton.on( 'click', function () {
+ function dumpTheModel () {
// linear model dump
var i, $li, element, html, annotations,
$ol = $( '<ol start="0"></ol>' );
@@ -420,7 +420,9 @@
getKids(
ve.instances[0].view.documentView.getDocumentNode() )
);
$( '#ve-dump' ).show();
- } );
+ }
+ dumpModelButton.on( 'click', dumpTheModel );
+ setInterval( dumpTheModel, 500 );
validateButton.on( 'click', function () {
var failed = false;
$( '.ve-ce-branchNode' ).each( function (
index, element ) {
diff --git a/modules/ve/ce/ve.ce.ContentBranchNode.js
b/modules/ve/ce/ve.ce.ContentBranchNode.js
index 10d365c..6a1383d 100644
--- a/modules/ve/ce/ve.ce.ContentBranchNode.js
+++ b/modules/ve/ce/ve.ce.ContentBranchNode.js
@@ -160,7 +160,7 @@
* @method
*/
ve.ce.ContentBranchNode.prototype.renderContents = function () {
- var i, len, node, rendered;
+ var i, len, node, rendered, oldContent, newContent;
if (
this.root instanceof ve.ce.DocumentNode &&
this.root.getSurface().isRenderingLocked()
@@ -172,6 +172,22 @@
this.root.getSurface().setContentBranchNodeChanged( true );
}
+ // Test for changes before wiping
+ oldContent = [];
+ newContent = [];
+ for ( i = 0, len = this.$element.length; i < len; i++ ) {
+ node = this.$element[i];
+ oldContent.push( node.textContent );
+ }
+ rendered = this.getRenderedContents();
+ for ( i = 0, len = rendered.length; i < len; i++ ) {
+ node = rendered[i];
+ newContent.push( node.textContent );
+ }
+ if ( oldContent.join( '' ) === newContent.join( '' ) ) {
+ return;
+ }
+
// Detach all child nodes from this.$element
for ( i = 0, len = this.$element.length; i < len; i++ ) {
node = this.$element[i];
@@ -181,7 +197,6 @@
}
// Reattach child nodes with the right annotations
- rendered = this.getRenderedContents();
for ( i = 0, len = rendered.length; i < len; i++ ) {
this.$element[0].appendChild( rendered[i] );
}
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index b7d8cb5..83aaa19 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -1243,7 +1243,7 @@
previousStart, nextStart, newRange,
previousData, nextData,
i, length, annotation, annotationIndex, dataString,
- annotationsLeft, annotationsRight,
+ annotationsLeft, annotationsRight, incomplete,
fromLeft = 0,
fromRight = 0,
nodeOffset = node.getModel().getOffset();
@@ -1370,13 +1370,15 @@
}
if ( data.length > 0 ) {
- this.changeModel(
- ve.dm.Transaction.newFromInsertion(
- this.documentView.model, nodeOffset + 1
+ fromLeft,
- data
- ),
- newRange
- );
+ incomplete = fromLeft + fromRight < previousData.length;
+ this.changeModel(
+ ve.dm.Transaction.newFromInsertion(
+ this.documentView.model, nodeOffset + 1 +
fromLeft,
+ data
+ ),
+ newRange,
+ incomplete
+ );
}
if ( fromLeft + fromRight < previousData.length ) {
this.changeModel(
@@ -1531,6 +1533,9 @@
* @method
*/
ve.ce.Surface.prototype.handleInsertion = function () {
+ if ( true ) {
+ return; // TODO: We're not ready for this line yet!
+ }
var slug, data, range, annotations, insertionAnnotations, placeholder,
selection = this.model.getSelection(), documentModel =
this.model.getDocument();
@@ -2105,15 +2110,17 @@
* @param {ve.dm.Transaction|ve.dm.Transaction[]|null} transactions One or
more transactions to
* process, or null to process none
* @param {ve.Range} new selection
+ * @param {boolean} [incomplete] Whether a following change is pending
* @throws {Error} If calls to this method are nested
*/
-ve.ce.Surface.prototype.changeModel = function ( transaction, range ) {
+ve.ce.Surface.prototype.changeModel = function ( transaction, range,
incomplete ) {
+ incomplete = !!incomplete;
if ( this.newModelSelection !== null ) {
throw new Error( 'Nested change of newModelSelection' );
}
this.newModelSelection = range;
try {
- this.model.change( transaction, range );
+ this.model.change( transaction, range, incomplete );
} finally {
this.newModelSelection = null;
}
diff --git a/modules/ve/dm/ve.dm.Document.js b/modules/ve/dm/ve.dm.Document.js
index 3b7541c..93e0075 100644
--- a/modules/ve/dm/ve.dm.Document.js
+++ b/modules/ve/dm/ve.dm.Document.js
@@ -290,14 +290,15 @@
*
* @method
* @param {ve.dm.Transaction} transaction Transaction to apply
+ * @param {boolean} incomplete Whether a subsequent commit is pending
* @fires transact
* @throws {Error} Cannot commit a transaction that has already been committed
*/
-ve.dm.Document.prototype.commit = function ( transaction ) {
+ve.dm.Document.prototype.commit = function ( transaction, incomplete ) {
if ( transaction.hasBeenApplied() ) {
throw new Error( 'Cannot commit a transaction that has already
been committed' );
}
- new ve.dm.TransactionProcessor( this, transaction ).process();
+ new ve.dm.TransactionProcessor( this, transaction ).process( incomplete
);
this.completeHistory.push( transaction );
this.emit( 'transact', transaction );
};
diff --git a/modules/ve/dm/ve.dm.DocumentSynchronizer.js
b/modules/ve/dm/ve.dm.DocumentSynchronizer.js
index 3f770a4..653985b 100644
--- a/modules/ve/dm/ve.dm.DocumentSynchronizer.js
+++ b/modules/ve/dm/ve.dm.DocumentSynchronizer.js
@@ -93,8 +93,9 @@
* @static
* @method
* @param {Object} action
+ * @param {boolean} incomplete Whether a subsequent synchronization is pending
*/
-ve.dm.DocumentSynchronizer.synchronizers.resize = function ( action ) {
+ve.dm.DocumentSynchronizer.synchronizers.resize = function ( action,
incomplete ) {
var node = action.node,
parent = node.getParent();
@@ -105,7 +106,7 @@
// Apply length change to tree
// No update event needed, adjustLength causes an update event
on its own
// FIXME however, any queued update event will still be
emitted, resulting in a duplicate
- node.adjustLength( action.adjustment );
+ node.adjustLength( action.adjustment, incomplete );
}
// Update adjustment
this.adjustment += action.adjustment;
@@ -275,8 +276,9 @@
* This method also clears both action and event queues.
*
* @method
+ * @param {boolean} incomplete Whether a subsequent synchronization is pending
*/
-ve.dm.DocumentSynchronizer.prototype.synchronize = function () {
+ve.dm.DocumentSynchronizer.prototype.synchronize = function ( incomplete ) {
var action,
event,
i;
@@ -284,7 +286,8 @@
for ( i = 0; i < this.actionQueue.length; i++ ) {
action = this.actionQueue[i];
if ( action.type in ve.dm.DocumentSynchronizer.synchronizers ) {
-
ve.dm.DocumentSynchronizer.synchronizers[action.type].call( this, action );
+
ve.dm.DocumentSynchronizer.synchronizers[action.type].call( this,
+ action, incomplete );
} else {
throw new Error( 'Invalid action type ' + action.type );
}
diff --git a/modules/ve/dm/ve.dm.Node.js b/modules/ve/dm/ve.dm.Node.js
index 59b720c..a5c405e 100644
--- a/modules/ve/dm/ve.dm.Node.js
+++ b/modules/ve/dm/ve.dm.Node.js
@@ -463,11 +463,12 @@
*
* @method
* @param {number} length Length of content
+ * @param {boolean} incomplete Whether called by synchronize with a subsequent
synchronize pending
* @fires lengthChange
* @fires update
* @throws {Error} Invalid content length error if length is less than 0
*/
-ve.dm.Node.prototype.setLength = function ( length ) {
+ve.dm.Node.prototype.setLength = function ( length, incomplete ) {
if ( length < 0 ) {
throw new Error( 'Length cannot be negative' );
}
@@ -481,7 +482,9 @@
}
// Emit events
this.emit( 'lengthChange', diff );
- this.emit( 'update' );
+ if ( !incomplete ) {
+ this.emit( 'update' );
+ }
};
/**
@@ -492,12 +495,13 @@
*
* @method
* @param {number} adjustment Amount to adjust length by
+ * @param {boolean} incomplete Whether called by synchronize with a subsequent
synchronize pending
* @fires lengthChange
* @fires update
* @throws {Error} Invalid adjustment error if resulting length is less than 0
*/
-ve.dm.Node.prototype.adjustLength = function ( adjustment ) {
- this.setLength( this.length + adjustment );
+ve.dm.Node.prototype.adjustLength = function ( adjustment, incomplete ) {
+ this.setLength( this.length + adjustment, incomplete );
};
/**
diff --git a/modules/ve/dm/ve.dm.Surface.js b/modules/ve/dm/ve.dm.Surface.js
index e21eda4..0cbc2cb 100644
--- a/modules/ve/dm/ve.dm.Surface.js
+++ b/modules/ve/dm/ve.dm.Surface.js
@@ -443,10 +443,11 @@
* @param {ve.dm.Transaction|ve.dm.Transaction[]|null} transactions One or
more transactions to
* process, or null to process none
* @param {ve.Range} [selection] Selection to apply
+ * @param {boolean} incomplete Whether a subsequent connected change is pending
* @fires contextChange
*/
-ve.dm.Surface.prototype.change = function ( transactions, selection ) {
- this.changeInternal( transactions, selection, false );
+ve.dm.Surface.prototype.change = function ( transactions, selection,
incomplete ) {
+ this.changeInternal( transactions, selection, false, incomplete );
};
/**
@@ -457,9 +458,11 @@
* @param {ve.dm.Transaction|ve.dm.Transaction[]|null} transactions
* @param {ve.Range} [selection] [selection]
* @param {boolean} [skipUndoStack=false] If true, do not modify the undo
stack. Used by undo/redo
+ * @param {boolean} incomplete Whether a subsequent synchronization is pending
* @fires contextChange
*/
-ve.dm.Surface.prototype.changeInternal = function ( transactions, selection,
skipUndoStack ) {
+ve.dm.Surface.prototype.changeInternal = function ( transactions, selection,
skipUndoStack,
+ incomplete ) {
var i, len, selectionAfter, selectionBefore = this.selection,
contextChange = false;
if ( !this.enabled ) {
@@ -481,7 +484,7 @@
this.newTransactions.push(
transactions[i] );
}
// The .commit() call below indirectly invokes
setSelection()
- this.documentModel.commit( transactions[i] );
+ this.documentModel.commit( transactions[i],
incomplete );
if (
transactions[i].hasElementAttributeOperations() ) {
contextChange = true;
}
diff --git a/modules/ve/dm/ve.dm.TransactionProcessor.js
b/modules/ve/dm/ve.dm.TransactionProcessor.js
index 80ff62f..fe9db9d 100644
--- a/modules/ve/dm/ve.dm.TransactionProcessor.js
+++ b/modules/ve/dm/ve.dm.TransactionProcessor.js
@@ -85,8 +85,9 @@
* When all operations are done being processed, the document will be
synchronized.
*
* @method
+ * @param {boolean} incomplete Whether a subsequent call to process is pending
*/
-ve.dm.TransactionProcessor.prototype.process = function () {
+ve.dm.TransactionProcessor.prototype.process = function ( incomplete ) {
var op;
// This loop is factored this way to allow operations to be skipped
over or executed
@@ -95,7 +96,7 @@
while ( ( op = this.nextOperation() ) ) {
this.executeOperation( op );
}
- this.synchronizer.synchronize();
+ this.synchronizer.synchronize( incomplete );
// Mark the transaction as committed or rolled back, as appropriate
this.transaction.markAsApplied();
--
To view, visit https://gerrit.wikimedia.org/r/102009
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ied06031e9f25ea6ac80c64035f04eef82ce46df0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
