jenkins-bot has submitted this change and it was merged.
Change subject: Enslave dm.Surface to dm.Document
......................................................................
Enslave dm.Surface to dm.Document
* Replace surface 'transact' event with 'documentUpdate' event
* Have surface listen for all document transactions and update selection
as appropriate (as well as emitting 'documentUpdate')
* Implement change() in terms of setSelection()
** Queue 'contextChange' events so contextChange is only emitted once
** Use this.transacting flag to prevent setSelection() (which is called
because the model emits transact events) from doing too much
** Behavioral change: lock/unlock now emitted separately for
transaction and selection changes
Change-Id: I44425d260ec70758f99d13f99e41f5c993d260c2
---
M modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
M modules/ve/dm/ve.dm.Surface.js
M modules/ve/test/dm/ve.dm.Surface.test.js
3 files changed, 201 insertions(+), 80 deletions(-)
Approvals:
Divec: Looks good to me, approved
jenkins-bot: Verified
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 736cb9b..7f483c4 100644
--- a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
+++ b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
@@ -708,10 +708,10 @@
* looks like wikitext, so it can trigger if the user types in a paragraph
that has pre-existing
* wikitext-like content.
*
- * This method is bound to the 'transact' event on the surface model, and
unbinds itself when
+ * This method is bound to the 'documentUpdate' event on the surface model,
and unbinds itself when
* the wikitext notification is displayed.
*
- * @param {ve.dm.Transaction[]} transactions
+ * @param {ve.dm.Transaction} transaction
*/
ve.init.mw.ViewPageTarget.prototype.checkForWikitextWarning = function () {
var text, doc = this.surface.getView().getDocument(),
@@ -733,7 +733,7 @@
}
);
this.surface.getModel().disconnect(
- this, { 'transact': 'checkForWikitextWarning' }
+ this, { 'documentUpdate': 'checkForWikitextWarning' }
);
}
};
@@ -970,7 +970,7 @@
'transact':
'clearSaveDialogDiff'
} );
target.surface.getModel().connect(
target, {
- 'transact':
'checkForWikitextWarning',
+ 'documentUpdate':
'checkForWikitextWarning',
'history':
'updateToolbarSaveButtonState'
} );
target.$.append( target.surface.$ );
diff --git a/modules/ve/dm/ve.dm.Surface.js b/modules/ve/dm/ve.dm.Surface.js
index 9ad1095..e7d912b 100644
--- a/modules/ve/dm/ve.dm.Surface.js
+++ b/modules/ve/dm/ve.dm.Surface.js
@@ -29,6 +29,12 @@
this.historyTrackingInterval = null;
this.insertionAnnotations = new ve.dm.AnnotationSet(
this.documentModel.getStore() );
this.enabled = true;
+ this.transacting = false;
+ this.queueingContextChanges = false;
+ this.contextChangeQueued = false;
+
+ // Events
+ this.documentModel.connect( this, { 'transact': 'onDocumentTransact' }
);
};
/* Inheritance */
@@ -53,6 +59,16 @@
/**
* @event transact
* @param {ve.dm.Transaction[]} transactions Transactions that have just been
processed
+ */
+
+/**
+ * @event documentUpdate
+ *
+ * Emitted when a transaction has been processed on the document and the
selection has been
+ * translated to account for that transaction. You should only use this event
if you need
+ * to access the selection; in most cases, you should use
{ve.dm.Document#event-transact}.
+ *
+ * @param {ve.dm.Transaction} tx Transaction that was processed on the document
*/
/**
@@ -289,84 +305,107 @@
};
/**
- * Apply a transactions and selection changes to the document.
+ * Start queueing up calls to {#emitContextChange} until
{#stopQueueingContextChanges} is called.
+ * While queueing is active, contextChanges are also collapsed, so if
{#emitContextChange} is called
+ * multiple times, only one contextChange event will be emitted by
{#stopQueueingContextChanges}.
+ *
+ * @example
+ * this.emitContextChange(); // emits immediately
+ * this.startQueueingContextChanges();
+ * this.emitContextChange(); // doesn't emit
+ * this.emitContextChange(); // doesn't emit
+ * this.stopQueueingContextChanges(); // emits one contextChange event
*
* @method
- * @param {ve.dm.Transaction|ve.dm.Transaction[]|null} transactions One or
more transactions to
- * process, or null to process none
- * @param {ve.Range|undefined} selection
- * @fires lock
- * @fires select
- * @fires transact
- * @fires contextChange
- * @fires unlock
+ * @private
*/
-ve.dm.Surface.prototype.change = function ( transactions, selection ) {
- if ( !this.enabled ) {
- return;
+ve.dm.Surface.prototype.startQueueingContextChanges = function () {
+ if ( !this.queueingContextChanges ) {
+ this.queueingContextChanges = true;
+ this.contextChangeQueued = false;
}
- var i, len, left, right, leftAnnotations, rightAnnotations,
insertionAnnotations,
+};
+
+/**
+ * Emit a contextChange event. If {#startQueueingContextChanges} has been
called, then the event
+ * is deferred until {#stopQueueingContextChanges} is called.
+ *
+ * @method
+ * @private
+ * @fires contextChange
+ */
+ve.dm.Surface.prototype.emitContextChange = function () {
+ if ( this.queueingContextChanges ) {
+ this.contextChangeQueued = true;
+ } else {
+ this.emit( 'contextChange' );
+ }
+};
+
+/**
+ * Stop queueing contextChange events. If {#emitContextChange} was called
previously, a contextChange
+ * event will now be emitted. Any future calls to {#emitContextChange} will
once again emit the
+ * event immediately.
+ *
+ * @method
+ * @private
+ * @fires contextChange
+ */
+ve.dm.Surface.prototype.stopQueueingContextChanges = function () {
+ if ( this.queueingContextChanges ) {
+ this.queueingContextChanges = false;
+ if ( this.contextChangeQueued ) {
+ this.contextChangeQueued = false;
+ this.emit( 'contextChange' );
+ }
+ }
+};
+
+/**
+ * Change the selection
+ *
+ * @param {ve.Range} selection New selection
+ *
+ * @fires lock
+ * @fires unlock
+ * @fires select
+ * @fires contextChange
+ */
+ve.dm.Surface.prototype.setSelection = function ( selection ) {
+ var left, right, leftAnnotations, rightAnnotations,
insertionAnnotations,
selectedNodes = {},
oldSelection = this.selection,
contextChange = false,
dataModelData = this.documentModel.data;
+ if ( !this.enabled ) {
+ return;
+ }
+
+ if ( this.transacting ) {
+ // Update the selection but don't do any processing
+ this.selection = selection;
+ return;
+ }
+
// Stop observation polling, things changing right now are known already
this.emit( 'lock' );
- // Process transactions and apply selection changes
- if ( transactions ) {
- if ( transactions instanceof ve.dm.Transaction ) {
- transactions = [transactions];
- }
- for ( i = 0, len = transactions.length; i < len; i++ ) {
- if ( !transactions[i].isNoOp() ) {
- this.truncateUndoStack();
- this.smallStack.push( transactions[i] );
- this.documentModel.commit( transactions[i] );
- if ( !selection ) {
- // translateRange only if selection is
not provided because otherwise we are
- // going to overwrite it
- this.selection =
transactions[i].translateRange( this.selection );
- }
- }
- }
+ // Detect if selected nodes changed
+ selectedNodes.start = this.documentModel.getNodeFromOffset(
selection.start );
+ if ( selection.getLength() ) {
+ selectedNodes.end = this.documentModel.getNodeFromOffset(
selection.end );
+ }
+ if (
+ selectedNodes.start !== this.selectedNodes.start ||
+ selectedNodes.end !== this.selectedNodes.end
+ ) {
+ contextChange = true;
}
- if ( selection ) {
- // Detect if selected nodes changed
- selectedNodes.start = this.documentModel.getNodeFromOffset(
selection.start );
- if ( selection.getLength() ) {
- selectedNodes.end =
this.documentModel.getNodeFromOffset( selection.end );
- }
- if (
- selectedNodes.start !== this.selectedNodes.start ||
- selectedNodes.end !== this.selectedNodes.end
- ) {
- contextChange = true;
- }
- this.selectedNodes = selectedNodes;
- this.selection = selection;
- }
-
- // Emit select event if selection range changed
- if ( !oldSelection || !oldSelection.equals( this.selection ) ) {
- this.emit( 'select', this.selection.clone() );
- }
-
- // Only emit a transact event if transactions were actually processed
- if ( transactions ) {
- this.emit( 'transact', transactions );
- // Detect context change, if not detected already, when element
attributes have changed
- if ( !contextChange ) {
- for ( i = 0, len = transactions.length; i < len; i++ ) {
- if (
transactions[i].hasElementAttributeOperations() ) {
- contextChange = true;
- break;
- }
- }
- }
- }
+ // Update state
+ this.selectedNodes = selectedNodes;
+ this.selection = selection;
// Figure out which annotations to use for insertions
if ( this.selection.isCollapsed() ) {
@@ -401,13 +440,81 @@
contextChange = true;
}
- // Only emit one context change event
+ // Emit events
+ if ( !oldSelection || !oldSelection.equals( this.selection ) ) {
+ this.emit( 'select', this.selection.clone() );
+ }
if ( contextChange ) {
- this.emit( 'contextChange' );
+ this.emitContextChange();
}
// Continue observation polling, we want to know about things that
change from here on out
this.emit( 'unlock' );
+};
+
+/**
+ * Apply a transactions and selection changes to the document.
+ *
+ * @method
+ * @param {ve.dm.Transaction|ve.dm.Transaction[]|null} transactions One or
more transactions to
+ * process, or null to process none
+ * @param {ve.Range|undefined} selection
+ * @fires lock
+ * @fires contextChange
+ * @fires unlock
+ */
+ve.dm.Surface.prototype.change = function ( transactions, selection ) {
+ var i, len, selectionAfter, selectionBefore = this.selection,
contextChange = false;
+
+ if ( !this.enabled ) {
+ return;
+ }
+
+ this.startQueueingContextChanges();
+
+ // Process transactions
+ if ( transactions ) {
+ if ( transactions instanceof ve.dm.Transaction ) {
+ transactions = [transactions];
+ }
+ this.transacting = true;
+ this.emit( 'lock' );
+ for ( i = 0, len = transactions.length; i < len; i++ ) {
+ if ( !transactions[i].isNoOp() ) {
+ this.truncateUndoStack();
+ this.smallStack.push( transactions[i] );
+ // The .commit() call below indirectly invokes
setSelection()
+ this.documentModel.commit( transactions[i] );
+ if (
transactions[i].hasElementAttributeOperations() ) {
+ contextChange = true;
+ }
+ }
+ }
+ this.emit( 'unlock' );
+ this.transacting = false;
+ }
+ selectionAfter = this.selection;
+
+ // Apply selection change
+ if ( selection ) {
+ this.setSelection( selection );
+ } else if ( transactions ) {
+ // Call setSelection() to trigger selection processing that was
bypassed earlier
+ this.setSelection( this.selection );
+ }
+
+ // If the selection changed while applying the transactions but not
while applying the
+ // selection change, setSelection() won't have emitted a 'select'
event. We don't want that
+ // to happen, so emit one anyway.
+ if ( !selectionBefore.equals( selectionAfter ) &&
selectionAfter.equals( this.selection ) ) {
+ this.emit( 'select', this.selection.clone() );
+ }
+
+ if ( contextChange ) {
+ this.emitContextChange();
+ }
+
+ this.stopQueueingContextChanges();
};
/**
@@ -499,3 +606,15 @@
}
return null;
};
+
+/**
+ * Respond to transactions processed on the document by translating the
selection and updating
+ * other state.
+ *
+ * @param {ve.dm.Transaction} tx Transaction that was processed
+ * @fires documentUpdate
+ */
+ve.dm.Surface.prototype.onDocumentTransact = function ( tx ) {
+ this.setSelection( tx.translateRange( this.selection ) );
+ this.emit( 'documentUpdate', tx );
+};
diff --git a/modules/ve/test/dm/ve.dm.Surface.test.js
b/modules/ve/test/dm/ve.dm.Surface.test.js
index bd99959..bd0df64 100644
--- a/modules/ve/test/dm/ve.dm.Surface.test.js
+++ b/modules/ve/test/dm/ve.dm.Surface.test.js
@@ -33,25 +33,27 @@
} );
QUnit.test( 'change', 3, function ( assert ) {
- var surface = new ve.dm.SurfaceStub(),
- tx = new ve.dm.Transaction(),
+ var tx, surface = new ve.dm.SurfaceStub(),
events = {
- 'transact': 0,
+ 'documentUpdate': 0,
'select': 0
};
- surface.on( 'transact', function () {
- events.transact++;
+ // docmentUpdate doesn't fire for no-op transactions, so make sure
there's something there
+ tx = ve.dm.Transaction.newFromInsertion( surface.getDocument(), 3, [
'i' ] );
+
+ surface.on( 'documentUpdate', function () {
+ events.documentUpdate++;
} );
surface.on( 'select', function () {
events.select++;
} );
- surface.change( tx );
- assert.deepEqual( events, { 'transact': 1, 'select': 0 }, 'transaction
without selection' );
+ surface.change( tx.clone() );
+ assert.deepEqual( events, { 'documentUpdate': 1, 'select': 0 },
'transaction without selection' );
surface.change( null, new ve.Range( 2, 2 ) );
- assert.deepEqual( events, { 'transact': 1, 'select': 1 }, 'selection
without transaction' );
- surface.change( tx, new ve.Range( 3, 3 ) );
- assert.deepEqual( events, { 'transact': 2, 'select': 2 }, 'transaction
and selection' );
+ assert.deepEqual( events, { 'documentUpdate': 1, 'select': 1 },
'selection without transaction' );
+ surface.change( tx.clone(), new ve.Range( 3, 3 ) );
+ assert.deepEqual( events, { 'documentUpdate': 2, 'select': 2 },
'transaction and selection' );
} );
QUnit.test( 'breakpoint', 7, function ( assert ) {
--
To view, visit https://gerrit.wikimedia.org/r/88485
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I44425d260ec70758f99d13f99e41f5c993d260c2
Gerrit-PatchSet: 15
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Divec <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
