Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/153709
Change subject: Followup 95ee357: make remapStoreIndexes() work again
......................................................................
Followup 95ee357: make remapStoreIndexes() work again
Its implementation relied on getAnnotationIndexesAtOffset() returning
the indexes array by reference, but since 95ee357 it returns a copy.
Instead of modifying the annotations array by reference, modify the
copy and save it back using the new setAnnotationIndexesAtOffset()
method.
Also add tests for this function, we evidently need them. This
code was sort of covered by the paste tests in ce.Surface but not
very well. One of the paste tests started failing mysteriously, but
only in PhantomJS and only in the MediaWiki integration with no skin
loaded. It turned out PhantomJS was wrapping things in
<span style="line-height: normal;" class="Apple-style-span">
which was then stripped by afterPaste. However, by the time it was
stripped it was already in the IVStore of the internal dm.Document
that afterPaste uses, and so the store index of the <b> annotation
that was not stripped would differ between the internal document
and the main document. This should have been resolved by remapping
the store indexes in the data before inserting it, but because
remapStoreIndexes() was broken this didn't happen, and the test failed.
Change-Id: I3c9a4f8417107bfcd208cb9333fbbd188e13bb80
---
M modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
M modules/ve/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
2 files changed, 142 insertions(+), 5 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/09/153709/1
diff --git a/modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
b/modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
index c72c994..157dfb6 100644
--- a/modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
+++ b/modules/ve/dm/lineardata/ve.dm.ElementLinearData.js
@@ -324,15 +324,28 @@
* @param {ve.dm.AnnotationSet} annotations Annotations to set
*/
ve.dm.ElementLinearData.prototype.setAnnotationsAtOffset = function ( offset,
annotations ) {
+ this.setAnnotationIndexesAtOffset( offset, this.getStore().indexes(
annotations.get() ) );
+};
+
+/**
+ * Set annotations' store indexes at a specified offset.
+ *
+ * Cleans up data structure if indexes array is empty.
+ *
+ * @method
+ * @param {number} offset Offset to set annotation indexes at
+ * @param {number[]} indexes Annotations' store indexes
+ */
+ve.dm.ElementLinearData.prototype.setAnnotationIndexesAtOffset = function (
offset, indexes ) {
var character, item = this.getData( offset ), isElement =
this.isElementData( offset );
- if ( !annotations.isEmpty() ) {
+ if ( indexes.length > 0 ) {
if ( isElement ) {
// New element annotation
- item.annotations = this.getStore().indexes(
annotations.get() );
+ item.annotations = indexes;
} else {
// New character annotation
character = this.getCharacterData( offset );
- this.setData( offset, [character,
this.getStore().indexes( annotations.get() )] );
+ this.setData( offset, [character, indexes] );
}
} else {
if ( isElement ) {
@@ -761,12 +774,11 @@
ve.dm.ElementLinearData.prototype.remapStoreIndexes = function ( mapping ) {
var i, ilen, j, jlen, indexes, nodeClass;
for ( i = 0, ilen = this.data.length; i < ilen; i++ ) {
- // Returns annotation indexes by reference. Use ignoreClose
- // to avoid mapping the annotations twice.
indexes = this.getAnnotationIndexesFromOffset( i, true );
for ( j = 0, jlen = indexes.length; j < jlen; j++ ) {
indexes[j] = mapping[indexes[j]];
}
+ this.setAnnotationIndexesAtOffset( i, indexes );
if ( this.isOpenElementData( i ) ) {
nodeClass = ve.dm.nodeFactory.lookup( this.getType( i )
);
nodeClass.static.remapStoreIndexes( this.data[i],
mapping );
diff --git a/modules/ve/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
b/modules/ve/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
index 8aa2b61..a3a09d9 100644
--- a/modules/ve/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
+++ b/modules/ve/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
@@ -1519,6 +1519,131 @@
}
} );
+QUnit.test( 'remapStoreIndexes', function ( assert ) {
+ var i, data,
+ cases = [
+ {
+ before: [
+ ['F', [0]],
+ ['o', [1]],
+ ['o', [2]]
+ ],
+ mapping: {
+ 0: 1,
+ 1: 2,
+ 2: 3
+ },
+ after: [
+ ['F', [1]],
+ ['o', [2]],
+ ['o', [3]]
+ ],
+ msg: 'Annotated text: increment indexes'
+ },
+ {
+ before: [
+ ['F', [0]],
+ ['o', [1]],
+ ['o', [2]]
+ ],
+ mapping: {
+ 0: 1,
+ 1: 0,
+ 2: 2
+ },
+ after: [
+ ['F', [1]],
+ ['o', [0]],
+ ['o', [2]]
+ ],
+ msg: 'Annotated text: swap 0 and 1'
+ },
+ {
+ before: [
+ ['F', [0, 1]],
+ ['o', [1, 2]],
+ ['o', [2, 3]]
+ ],
+ mapping: {
+ 0: 3,
+ 1: 2,
+ 2: 1,
+ 3: 0
+ },
+ after: [
+ ['F', [3, 2]],
+ ['o', [2, 1]],
+ ['o', [1, 0]]
+ ],
+ msg: 'Annotated text: multiple annotations
mapped, order preserved'
+ },
+ {
+ before: [
+ { type: 'alienInline', annotations: [0]
},
+ { type: '/alienInline' }
+ ],
+ mapping: {
+ 0: 1,
+ 1: 0
+ },
+ after: [
+ { type: 'alienInline', annotations: [1]
},
+ { type: '/alienInline' }
+ ],
+ msg: 'Annotated node'
+ },
+ {
+ before: [
+ { type: 'paragraph' },
+ 'F',
+ ['o', [2, 1, 3]],
+ ['o', [4]],
+ { type: 'alienInline', annotations: [5,
0] },
+ { type: '/alienInline' },
+ ['B', [5, 0]],
+ ['a', [7]],
+ 'r',
+ { type: 'alienInline', annotations: [6]
},
+ { type: '/alienInline' },
+ { type: '/paragraph' }
+ ],
+ mapping: {
+ 0: 1,
+ 1: 4,
+ 2: 2,
+ 3: 8,
+ 4: 5,
+ 5: 7,
+ 6: 3,
+ 7: 6,
+ 8: 0
+ },
+ after: [
+ { type: 'paragraph' },
+ 'F',
+ ['o', [2, 4, 8]],
+ ['o', [5]],
+ { type: 'alienInline', annotations: [7,
1] },
+ { type: '/alienInline' },
+ ['B', [7, 1]],
+ ['a', [6]],
+ 'r',
+ { type: 'alienInline', annotations: [3]
},
+ { type: '/alienInline' },
+ { type: '/paragraph' }
+ ],
+ msg: 'Paragraph with mix of unannotated text,
annotated text and annotated nodes'
+ }
+ ];
+
+ QUnit.expect( cases.length );
+ for ( i = 0; i < cases.length; i++ ) {
+ data = new ve.dm.ElementLinearData( new
ve.dm.IndexValueStore(), cases[i].before );
+ data.remapStoreIndexes( cases[i].mapping );
+ assert.deepEqual( data.data, cases[i].after, cases[i].msg );
+ }
+} );
+
// TODO: ve.dm.ElementLinearData.static.compareUnannotated
// TODO: ve.dm.ElementLinearData#getAnnotationIndexesFromOffset
// TODO: ve.dm.ElementLinearData#setAnnotationsAtOffset
--
To view, visit https://gerrit.wikimedia.org/r/153709
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3c9a4f8417107bfcd208cb9333fbbd188e13bb80
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
