jenkins-bot has submitted this change and it was merged.
Change subject: Typing into an annotation next to a word break keeps annotation
......................................................................
Typing into an annotation next to a word break keeps annotation
Logic was failing because we were passing the index of the annotation
within the AnnotationSet, instead of the index within the Store, to
containsIndex().
Bug: 54332
Change-Id: Ibfd9abe6e4b44d9db744e0c5019418eee12f84a4
---
M modules/ve/ce/ve.ce.Surface.js
M modules/ve/dm/ve.dm.Surface.js
M modules/ve/test/ce/ve.ce.Surface.test.js
3 files changed, 76 insertions(+), 6 deletions(-)
Approvals:
Krinkle: Looks good to me, but someone else must approve
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index c3d4d68..e1a3fa0 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -987,7 +987,7 @@
* @see ve.ce.SurfaceObserver#pollOnce
*
* @method
- * @param {HTMLElement} node DOM node the change occured in
+ * @param {ve.ce.Node} node CE node the change occured in
* @param {Object} previous Old data
* @param {Object} previous.text Old plain text content
* @param {Object} previous.hash Old DOM hash
@@ -1001,7 +1001,7 @@
var data, range, len, annotations, offsetDiff, lengthDiff,
sameLeadingAndTrailing,
previousStart, nextStart, newRange,
previousData, nextData,
- i, length, annotation, dataString,
+ i, length, annotation, annotationIndex, dataString,
annotationsLeft, annotationsRight,
fromLeft = 0,
fromRight = 0,
@@ -1102,17 +1102,18 @@
annotationsRight =
this.model.getDocument().data.getAnnotationsFromOffset( nodeOffset + 1 +
previousData.length - fromRight );
for ( i = 0, length = annotations.getLength(); i < length; i++
) {
annotation = annotations.get( i );
+ annotationIndex = annotations.getIndex( i );
if ( annotation.constructor.static.splitOnWordbreak ) {
dataString = new ve.dm.DataString( nextData );
if (
// if no annotation to the right, check
for wordbreak
(
-
!annotationsRight.containsIndex( i ) &&
+
!annotationsRight.containsIndex( annotationIndex ) &&
unicodeJS.wordbreak.isBreak(
dataString, fromLeft )
) ||
// if no annotation to the left, check
for wordbreak
(
- !annotationsLeft.containsIndex(
i ) &&
+ !annotationsLeft.containsIndex(
annotationIndex ) &&
unicodeJS.wordbreak.isBreak(
dataString, nextData.length - fromRight )
)
) {
diff --git a/modules/ve/dm/ve.dm.Surface.js b/modules/ve/dm/ve.dm.Surface.js
index b6aec21..04283fd 100644
--- a/modules/ve/dm/ve.dm.Surface.js
+++ b/modules/ve/dm/ve.dm.Surface.js
@@ -140,7 +140,7 @@
* Get a list of all history states.
*
* @method
- * @returns {Array[]} List of transaction stacks
+ * @returns {Object[]} List of transaction stacks
*/
ve.dm.Surface.prototype.getHistory = function () {
if ( this.smallStack.length > 0 ) {
diff --git a/modules/ve/test/ce/ve.ce.Surface.test.js
b/modules/ve/test/ce/ve.ce.Surface.test.js
index 6fb0e2f..eff0e02 100644
--- a/modules/ve/test/ce/ve.ce.Surface.test.js
+++ b/modules/ve/test/ce/ve.ce.Surface.test.js
@@ -113,6 +113,76 @@
}
} );
+QUnit.test( 'onContentChange', function ( assert ) {
+ var i,
+ cases = [
+ {
+ 'prevHtml': '<p><a href="Foo">A</a><a
href="Bar">FooX?</a></p>',
+ 'prevRange': new ve.Range( 5, 6 ),
+ 'nextHtml': '<p><a href="Foo">A</a><a
href="Bar">FooB?</a></p>',
+ 'nextRange': new ve.Range( 6 ),
+ 'expectedOps': [
+ [
+ { 'type': 'retain', 'length': 5
},
+ {
+ 'type': 'replace',
+ 'insert': [ ['B', [1]]
],
+ 'remove': []
+ },
+ { 'type': 'retain', 'length': 5
}
+ ],
+ [
+ { 'type': 'retain', 'length': 6
},
+ {
+ 'type': 'replace',
+ 'insert': [],
+ 'remove': [ ['X', [1]] ]
+ },
+ { 'type': 'retain', 'length': 4
}
+ ]
+ ],
+ 'msg': 'Replace into non-zero annotation next
to word break'
+ }
+ ];
+
+ QUnit.expect( cases.length * 2 );
+
+ function testRunner( prevHtml, prevRange, nextHtml, nextRange,
expectedOps, expectedRange, msg ) {
+ var txs, i, ops,
+ surface = ve.test.utils.createSurfaceFromHtml( prevHtml
),
+ view =
surface.getView().getDocument().getDocumentNode().children[0],
+ prevNode = $( prevHtml )[0],
+ nextNode = $( nextHtml )[0],
+ prev = {
+ 'text': ve.ce.getDomText( prevNode ),
+ 'hash': ve.ce.getDomHash( prevNode ),
+ 'range': prevRange
+ },
+ next = {
+ 'text': ve.ce.getDomText( nextNode ),
+ 'hash': ve.ce.getDomHash( nextNode ),
+ 'range': nextRange
+ };
+
+ surface.getView().onContentChange( view, prev, next );
+ txs = surface.getModel().getHistory()[0].stack;
+ ops = [];
+ for ( i = 0; i < txs.length; i++ ) {
+ ops.push( txs[i].getOperations() );
+ }
+ assert.deepEqual( ops, expectedOps, msg + ': operations' );
+ assert.deepEqual( surface.getModel().getSelection(),
expectedRange, msg + ': range' );
+ }
+
+ for ( i = 0; i < cases.length; i++ ) {
+ testRunner(
+ cases[i].prevHtml, cases[i].prevRange,
cases[i].nextHtml, cases[i].nextRange,
+ cases[i].expectedOps, cases[i].expectedRange ||
cases[i].nextRange, cases[i].msg
+ );
+ }
+
+} );
+
/* Methods with return values */
// TODO: ve.ce.Surface.static.getClipboardHash
// TODO: ve.ce.Surface#hasSlugAtOffset
@@ -152,7 +222,6 @@
// TODO: ve.ce.Surface#onDocumentCompositionEnd
// TODO: ve.ce.Surface#onChange
// TODO: ve.ce.Surface#onSelectionChange
-// TODO: ve.ce.Surface#onContentChange
// TODO: ve.ce.Surface#onLock
// TODO: ve.ce.Surface#onUnlock
// TODO: ve.ce.Surface#startRelocation
--
To view, visit https://gerrit.wikimedia.org/r/86235
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibfd9abe6e4b44d9db744e0c5019418eee12f84a4
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
