Divec has uploaded a new change for review.
https://gerrit.wikimedia.org/r/290425
Change subject: WIP: Calculate model ranges on demand, not RangeState creation
......................................................................
WIP: Calculate model ranges on demand, not RangeState creation
Change-Id: I244dfad38348b279a51b7233014566d4dd33f4b3
---
M src/ce/keydownhandlers/ve.ce.LinearArrowKeyDownHandler.js
M src/ce/ve.ce.RangeState.js
M src/ce/ve.ce.Surface.js
M src/ce/ve.ce.SurfaceObserver.js
M src/ce/ve.ce.js
M src/ve.SelectionState.js
6 files changed, 76 insertions(+), 200 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/25/290425/1
diff --git a/src/ce/keydownhandlers/ve.ce.LinearArrowKeyDownHandler.js
b/src/ce/keydownhandlers/ve.ce.LinearArrowKeyDownHandler.js
index f57abf6..24faf2b 100644
--- a/src/ce/keydownhandlers/ve.ce.LinearArrowKeyDownHandler.js
+++ b/src/ce/keydownhandlers/ve.ce.LinearArrowKeyDownHandler.js
@@ -186,7 +186,10 @@
} else {
// Check where the range has moved to
surface.surfaceObserver.pollOnceNoCallback();
- newRange = new ve.Range(
surface.surfaceObserver.getRange().to );
+ newRange = new ve.Range( ve.ce.getOffset(
+ surface.nativeSelection.focusNode,
+ surface.nativeSelection.focusOffset
+ ) );
}
// Adjust range to use old anchor, if necessary
diff --git a/src/ce/ve.ce.RangeState.js b/src/ce/ve.ce.RangeState.js
index 27a7ef6..0569120 100644
--- a/src/ce/ve.ce.RangeState.js
+++ b/src/ce/ve.ce.RangeState.js
@@ -34,11 +34,6 @@
this.contentChanged = false;
/**
- * @property {ve.Range|null} veRange The current selection range
- */
- this.veRange = null;
-
- /**
* @property {ve.ce.BranchNode|null} node The current branch node
*/
this.node = null;
@@ -100,10 +95,8 @@
if ( selection.equalsSelection( oldSelection ) ) {
// No change; use old values for speed
this.selectionChanged = false;
- this.veRange = old && old.veRange;
} else {
this.selectionChanged = true;
- this.veRange = ve.ce.veRangeFromSelection( selection );
}
anchorNodeChanged = oldSelection.anchorNode !== selection.anchorNode;
@@ -119,7 +112,6 @@
// Check this node belongs to our document
if ( this.node && this.node.root !== documentNode ) {
this.node = null;
- this.veRange = null;
}
}
}
@@ -167,3 +159,12 @@
// misleadingly different from when the selection was saved).
this.misleadingSelection = selection;
};
+
+/**
+ * Whether the selection is null
+ *
+ * @return {boolean} Whether the selection is null
+ */
+ve.ce.RangeState.prototype.isNullSelection = function () {
+ return this.misleadingSelection.isNull();
+};
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index e1dcc5e..1f74770 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -2618,7 +2618,7 @@
*/
ve.ce.Surface.prototype.handleObservedChanges = function ( oldState, newState
) {
- var newSelection, transaction, removedUnicorns,
+ var newSelection, transaction, oldRange, removedUnicorns,
activeNode, coveringRange, nodeRange, containsStart,
containsEnd,
surface = this,
dmDoc = this.getModel().getDocument(),
@@ -2632,6 +2632,7 @@
newState.node.unicornAnnotations
);
if ( transaction ) {
+ oldRange = ve.ce.veRangeFromSelection(
oldState.misleadingSelection );
this.incRenderLock();
try {
this.changeModel( transaction );
@@ -2651,21 +2652,22 @@
oldState.node.root &&
oldState.node instanceof ve.ce.ContentBranchNode
) {
+ oldRange = ve.ce.veRangeFromSelection(
oldState.misleadingSelection );
oldState.node.renderContents();
}
-
- if ( newState.selectionChanged && !(
- // Ignore when the newRange is just a flipped oldRange
- oldState &&
- oldState.veRange &&
- newState.veRange &&
- !newState.veRange.isCollapsed() &&
- oldState.veRange.equalsSelection( newState.veRange )
- ) ) {
- if ( newState.veRange ) {
- newSelection = new ve.dm.LinearSelection( dmDoc,
newState.veRange );
- } else {
+ if (
+ newState.selectionChanged &&
+ // Ignore when the new range is just a flipped old range
+ newState.misleadingSelection &&
+ newState.misleadingSelection.equalsRange(
oldState.misleadingSelection )
+ ) {
+ if ( newState.isNullSelection() ) {
newSelection = new ve.dm.NullSelection( dmDoc );
+ } else {
+ newSelection = new ve.dm.LinearSelection(
+ dmDoc,
+ ve.ce.veRangeFromSelection(
newState.misleadingSelection )
+ );
}
this.incRenderLock();
try {
@@ -2685,16 +2687,22 @@
nodeRange = activeNode.getRange();
containsStart = nodeRange.containsRange( new ve.Range(
coveringRange.start ) );
containsEnd = nodeRange.containsRange( new ve.Range(
coveringRange.end ) );
- // If the range starts xor ends in the active node, but
not both, then it must
- // span an active node boundary, so fixup.
+ // If the range starts or ends in the active node, but
not both, then it must
+ // span an active node boundary, so revert the
selection.
/*jshint bitwise: false*/
if ( containsStart ^ containsEnd ) {
- newSelection = oldState && oldState.veRange ?
- new ve.dm.LinearSelection( dmDoc,
oldState.veRange ) :
- new ve.dm.NullSelection( dmDoc );
+ if ( oldState && !oldState.isNullSelection() ) {
+ if ( !oldRange ) {
+ oldRange =
ve.ce.veRangeFromSelection( oldState.misleadingSelection );
+ }
+ // TODO translate oldRange if
newState.contentChanged?
+ newSelection = new
ve.dm.LinearSelection( dmDoc, oldRange );
+ } else {
+ newSelection = new ve.dm.NullSelection(
dmDoc );
+ }
setTimeout( function () {
surface.changeModel( null, newSelection
);
- surface .showModelSelection();
+ surface.showModelSelection();
} );
}
/*jshint bitwise: true*/
diff --git a/src/ce/ve.ce.SurfaceObserver.js b/src/ce/ve.ce.SurfaceObserver.js
index 4c9d9f1..c05dccc 100644
--- a/src/ce/ve.ce.SurfaceObserver.js
+++ b/src/ce/ve.ce.SurfaceObserver.js
@@ -184,17 +184,3 @@
ve.ce.SurfaceObserver.prototype.setTimeout = function ( callback, timeout ) {
return setTimeout( callback, timeout );
};
-
-/**
- * Get the range last observed.
- *
- * Used when you have just polled, but don't want to wait for a 'rangeChange'
event.
- *
- * @return {ve.Range|null} Range
- */
-ve.ce.SurfaceObserver.prototype.getRange = function () {
- if ( !this.rangeState ) {
- return null;
- }
- return this.rangeState.veRange;
-};
diff --git a/src/ce/ve.ce.js b/src/ce/ve.ce.js
index 44ed5ea..27884ad 100644
--- a/src/ce/ve.ce.js
+++ b/src/ce/ve.ce.js
@@ -163,6 +163,8 @@
/**
* Gets the linear offset from a given DOM node and offset within it.
*
+ * N.B. The model must be in sync with the view for this to work
+ *
* @method
* @param {HTMLElement} domNode DOM node
* @param {number} domOffset DOM offset within the DOM node
@@ -421,7 +423,9 @@
};
/**
- * Find the DM range of a DOM selection
+ * Find the DM range of a DOM selection.
+ *
+ * N.B. The model must be in sync with the view for this to work
*
* @param {Object} selection DOM-selection-like object
* @param {Node} selection.anchorNode
@@ -452,161 +456,6 @@
node = node.parentNode;
}
return $( node ).closest( '.ve-ce-linkAnnotation' )[ 0 ];
-};
-
-/**
- * Analyse a DOM content change to build a Transaction
- *
- * Content changes have oldState.node === newState.node and
newState.contentChanged === true .
- * Annotations are inferred heuristically from plaintext to do what the user
intended.
- * TODO: treat more changes as simple (not needing a re-render); see
- * https://phabricator.wikimedia.org/T114260 .
- *
- * @method
- * @param {ve.ce.RangeState} oldState The prior range state
- * @param {ve.ce.RangeState} newState The changed range state
- *
- * @return {Object} Results of analysis
- * @return {ve.dm.Transaction} return.transaction Transaction corresponding to
the DOM content change
- * @return {ve.dm.Selection} return.selection Changed selection to apply
(TODO: unsafe / useless?)
- * @return {boolean} return.rerender Whether the DOM needs rerendering after
applying the transaction
- */
-ve.ce.modelChangeFromContentChange = function ( oldState, newState ) {
- var data, len, annotations, bothCollapsed, oldStart, newStart,
replacementRange,
- fromLeft = 0,
- fromRight = 0,
- // It is guaranteed that oldState.node === newState.node , so
just call it 'node'
- node = newState.node,
- nodeOffset = node.getModel().getOffset(),
- oldText = oldState.text,
- newText = newState.text,
- oldRange = oldState.veRange,
- newRange = newState.veRange,
- oldData = oldText.split( '' ),
- newData = newText.split( '' ),
- lengthDiff = newText.length - oldText.length,
- dmDoc = node.getModel().getDocument(),
- modelData = dmDoc.data;
-
- bothCollapsed = oldRange.isCollapsed() && newRange.isCollapsed();
- oldStart = oldRange.start - nodeOffset - 1;
- newStart = newRange.start - nodeOffset - 1;
-
- // If the only change is an insertion just before the new cursor, then
apply a
- // single insertion transaction, using the annotations from the old
start
- // position (accounting for whether the cursor was before or after an
annotation
- // boundary)
- if (
- bothCollapsed &&
- lengthDiff > 0 &&
- oldText.slice( 0, oldStart ) === newText.slice( 0, oldStart ) &&
- oldText.slice( oldStart ) === newText.slice( newStart )
- ) {
- data = newData.slice( oldStart, newStart );
- if ( node.unicornAnnotations ) {
- annotations = node.unicornAnnotations;
- } else {
- annotations =
modelData.getInsertionAnnotationsFromRange(
- oldRange,
- oldState.focusIsAfterAnnotationBoundary
- );
- }
-
- if ( annotations.getLength() ) {
- ve.dm.Document.static.addAnnotationsToData( data,
annotations );
- }
-
- return {
- transaction: ve.dm.Transaction.newFromInsertion(
- dmDoc,
- oldRange.start,
- data
- ),
- selection: new ve.dm.LinearSelection( dmDoc, newRange ),
- rerender: false
- };
- }
-
- // If the only change is a removal touching the old cursor position,
then apply
- // a single removal transaction.
- if (
- bothCollapsed &&
- lengthDiff < 0 &&
- oldText.slice( 0, newStart ) === newText.slice( 0, newStart ) &&
- oldText.slice( newStart - lengthDiff ) === newText.slice(
newStart )
- ) {
- return {
- transaction: ve.dm.Transaction.newFromRemoval(
- dmDoc,
- new ve.Range( newRange.start, newRange.start -
lengthDiff )
- ),
- selection: new ve.dm.LinearSelection( dmDoc, newRange ),
- rerender: false
- };
- }
-
- // Complex change (either removal+insertion or insertion not just
before new cursor)
- // 1. Count unchanged characters from left and right;
- // 2. Assume that the minimal changed region indicates the replacement
made by the user;
- // 3. Hence guess how to map annotations.
- // N.B. this logic can go wrong; e.g. this code will see slice->slide
and
- // assume that the user changed 'c' to 'd', but the user could instead
have changed 'ic'
- // to 'id', which would map annotations differently.
-
- len = Math.min( oldData.length, newData.length );
-
- while ( fromLeft < len && oldData[ fromLeft ] === newData[ fromLeft ] )
{
- ++fromLeft;
- }
-
- while (
- fromRight < len - fromLeft &&
- oldData[ oldData.length - 1 - fromRight ] ===
- newData[ newData.length - 1 - fromRight ]
- ) {
- ++fromRight;
- }
- replacementRange = new ve.Range(
- nodeOffset + 1 + fromLeft,
- nodeOffset + 1 + oldData.length - fromRight
- );
- data = newData.slice( fromLeft, newData.length - fromRight );
-
- if ( node.unicornAnnotations ) {
- // This CBN is unicorned. Use the stored annotations.
- annotations = node.unicornAnnotations;
- } else {
- // Guess the annotations from the (possibly empty) range being
replaced.
- //
- // Still consider focusIsAfterAnnotationBoundary, even though
the change is
- // not necessarily at the cursor: assume the old focus was
inside the same
- // DOM text node as the insertion, and therefore has the same
annotations.
- // Otherwise, when using an IME that selects inserted text,
this code path
- // can cause an annotation discrepancy that triggers an
unwanted re-render,
- // closing the IME (For example, when typing at the start of
<p><i>x</i></p>
- // in Windows 8.1 Korean on IE11).
- annotations = modelData.getInsertionAnnotationsFromRange(
- replacementRange,
- oldRange.isCollapsed() &&
oldState.focusIsAfterAnnotationBoundary
- );
- }
- if ( annotations.getLength() ) {
- ve.dm.Document.static.addAnnotationsToData( data, annotations );
- }
- if ( newRange.isCollapsed() ) {
- // TODO: Remove this, or comment why it's necessary
- // (When wouldn't we be at a cursor offset?)
- newRange = new ve.Range( dmDoc.getNearestCursorOffset(
newRange.start, 1 ) );
- }
- return {
- transaction: ve.dm.Transaction.newFromReplacement(
- dmDoc,
- replacementRange,
- data
- ),
- selection: new ve.dm.LinearSelection( dmDoc, newRange ),
- rerender: true
- };
};
/**
diff --git a/src/ve.SelectionState.js b/src/ve.SelectionState.js
index 9db1434..4e3a61e 100644
--- a/src/ve.SelectionState.js
+++ b/src/ve.SelectionState.js
@@ -68,6 +68,15 @@
/* Methods */
/**
+ * Whether the selection is null
+ *
+ * @return {boolean} true if the focusNode is null (in which case the
anchorNode should be null too)
+ */
+ve.SelectionState.prototype.isNull = function () {
+ return this.focusNode === null;
+};
+
+/**
* Returns the selection with the anchor and focus swapped
*
* @return {ve.SelectionState} selection with anchor/focus swapped.
Object-identical to this if isCollapsed
@@ -87,10 +96,10 @@
};
/**
- * Whether the selection represents is the same range as another DOM
Selection-like object
+ * Whether this and another DOM Selection-like object have the same anchor and
the same focus
*
* @param {Object} other DOM Selection-like object
- * @return {boolean} True if the anchors/focuses are equal (including null)
+ * @return {boolean} True if the anchors and focuses are equal (including null)
*/
ve.SelectionState.prototype.equalsSelection = function ( other ) {
return this.anchorNode === other.anchorNode &&
@@ -100,6 +109,26 @@
};
/**
+ * Whether this and another DOM Selection-like object have the same
anchor/focus (possibly flipped)
+ *
+ * @param {Object} other DOM Selection-like object
+ * @return {boolean} True if the anchors and focuses are equal (including
null) or equal flipped
+ */
+ve.SelectionState.prototype.equalsRange = function ( other ) {
+ return (
+ this.anchorNode === other.anchorNode &&
+ this.anchorOffset === other.anchorOffset &&
+ this.focusNode === other.focusNode &&
+ this.focusOffset === other.focusOffset
+ ) || (
+ this.anchorNode === other.focusNode &&
+ this.anchorOffset === other.focusOffset &&
+ this.focusNode === other.anchorNode &&
+ this.focusOffset === other.anchorOffset
+ );
+};
+
+/**
* Get a range representation of the selection
*
* N.B. Range objects do not show whether the selection is backwards
--
To view, visit https://gerrit.wikimedia.org/r/290425
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I244dfad38348b279a51b7233014566d4dd33f4b3
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <da...@troi.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
