jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/343458 )
Change subject: When indexing originalDomElements for ve.dm.Annotations,
disregard child nodes
......................................................................
When indexing originalDomElements for ve.dm.Annotations, disregard child nodes
Bug: T160839
Change-Id: I1aec6072452051990f1147e4617739620c7b251f
---
M src/dm/ve.dm.Converter.js
M tests/ce/ve.ce.Surface.test.js
M tests/ce/ve.ce.TextState.test.js
M tests/dm/lineardata/ve.dm.ElementLinearData.test.js
M tests/dm/ve.dm.example.js
5 files changed, 107 insertions(+), 56 deletions(-)
Approvals:
Esanders: Looks good to me, but someone else must approve
jenkins-bot: Verified
Jforrester: Looks good to me, approved
diff --git a/src/dm/ve.dm.Converter.js b/src/dm/ve.dm.Converter.js
index 3a5c68f..745f6bb 100644
--- a/src/dm/ve.dm.Converter.js
+++ b/src/dm/ve.dm.Converter.js
@@ -402,7 +402,8 @@
* @return {Object|Array|null} Data element or array of linear model data, or
null to alienate
*/
ve.dm.Converter.prototype.createDataElements = function ( modelClass,
domElements ) {
- var dataElements = modelClass.static.toDataElement( domElements, this );
+ var serializer,
+ dataElements = modelClass.static.toDataElement( domElements,
this );
if ( !dataElements ) {
return null;
@@ -411,7 +412,18 @@
dataElements = [ dataElements ];
}
if ( dataElements.length ) {
- dataElements[ 0 ].originalDomElementsIndex = this.store.index(
domElements, domElements.map( ve.getNodeHtml ).join( '' ) );
+ if ( modelClass.prototype instanceof ve.dm.Annotation ) {
+ serializer = function ( node ) {
+ // Do not include childNodes; see T160839
+ return node.cloneNode( false ).outerHTML;
+ };
+ } else {
+ serializer = ve.getNodeHtml;
+ }
+ dataElements[ 0 ].originalDomElementsIndex = this.store.index(
+ domElements,
+ domElements.map( serializer ).join( '' )
+ );
}
return dataElements;
};
diff --git a/tests/ce/ve.ce.Surface.test.js b/tests/ce/ve.ce.Surface.test.js
index b782245..53aa686 100644
--- a/tests/ce/ve.ce.Surface.test.js
+++ b/tests/ce/ve.ce.Surface.test.js
@@ -1068,7 +1068,7 @@
QUnit.test( 'handleObservedChanges (content changes)', function ( assert ) {
var i,
- linkIndex = 'h4601de4ee174fedd',
+ linkIndex = 'h3f6906f71a963fc3',
cases = [
{
prevHtml: '<p></p>',
@@ -1140,7 +1140,7 @@
{ type: 'retain', length: 2 },
{
type: 'replace',
- insert: [ [ 'Y', [
'h3f03d2abae6ddc0d' ] ] ],
+ insert: [ [ 'Y', [
'hd72ee073faddca4e' ] ] ],
remove: [],
insertedDataOffset: 0,
insertedDataLength: 1
diff --git a/tests/ce/ve.ce.TextState.test.js b/tests/ce/ve.ce.TextState.test.js
index 34e064d..f0b8563 100644
--- a/tests/ce/ve.ce.TextState.test.js
+++ b/tests/ce/ve.ce.TextState.test.js
@@ -24,7 +24,7 @@
{ type: 'retain', length: 5 },
{
type: 'replace',
- remove: [ [ 'b', [ annIndex( 'b', 'bar'
) ] ], [ 'a', [ annIndex( 'b', 'bar' ) ] ], [ 'r', [ annIndex( 'b', 'bar' ) ] ]
],
+ remove: [ [ 'b', [ annIndex( 'b' ) ] ],
[ 'a', [ annIndex( 'b' ) ] ], [ 'r', [ annIndex( 'b' ) ] ] ],
insert: [ 'b', 'a', 'r' ],
insertedDataOffset: 0,
insertedDataLength: 3
@@ -42,7 +42,7 @@
{
type: 'replace',
remove: [],
- insert: [ [ 'r', [ annIndex( 'b', 'ba'
) ] ] ],
+ insert: [ [ 'r', [ annIndex( 'b' ) ] ]
],
insertedDataOffset: 0,
insertedDataLength: 1
},
@@ -76,7 +76,7 @@
{
type: 'replace',
remove: [],
- insert: [ [ 'z', [ annIndex( 'b', 'y' )
] ] ],
+ insert: [ [ 'z', [ annIndex( 'b' ) ] ]
],
insertedDataOffset: 0,
insertedDataLength: 1
},
@@ -129,9 +129,9 @@
type: 'replace',
remove: [ 'b', 'a', 'r' ],
insert: [
- [ 'b', [ annIndex( 'u', 'baz'
), boldIndex ] ],
- [ 'a', [ annIndex( 'u', 'baz'
), boldIndex ] ],
- [ 'r', [ annIndex( 'u', 'baz'
), boldIndex ] ]
+ [ 'b', [ annIndex( 'u' ),
boldIndex ] ],
+ [ 'a', [ annIndex( 'u' ),
boldIndex ] ],
+ [ 'r', [ annIndex( 'u' ),
boldIndex ] ]
],
insertedDataOffset: 0,
insertedDataLength: 3
@@ -149,14 +149,14 @@
{
type: 'replace',
remove: [
- [ 'b', [ annIndex( 'i', 'foo
<b>bar</b> baz' ), annIndex( 'b', 'bar' ) ] ],
- [ 'a', [ annIndex( 'i', 'foo
<b>bar</b> baz' ), annIndex( 'b', 'bar' ) ] ],
- [ 'r', [ annIndex( 'i', 'foo
<b>bar</b> baz' ), annIndex( 'b', 'bar' ) ] ]
+ [ 'b', [ annIndex( 'i' ),
annIndex( 'b' ) ] ],
+ [ 'a', [ annIndex( 'i' ),
annIndex( 'b' ) ] ],
+ [ 'r', [ annIndex( 'i' ),
annIndex( 'b' ) ] ]
],
insert: [
- [ 'b', [ annIndex( 'i', 'foo
<b>bar</b> baz' ) ] ],
- [ 'a', [ annIndex( 'i', 'foo
<b>bar</b> baz' ) ] ],
- [ 'r', [ annIndex( 'i', 'foo
<b>bar</b> baz' ) ] ]
+ [ 'b', [ annIndex( 'i' ) ] ],
+ [ 'a', [ annIndex( 'i' ) ] ],
+ [ 'r', [ annIndex( 'i' ) ] ]
],
insertedDataOffset: 0,
insertedDataLength: 3
@@ -174,7 +174,7 @@
{
type: 'replace',
remove: [],
- insert: [ [ 'r', [ annIndex( 'i', 'foo
<b>ba</b> baz' ), annIndex( 'b', 'ba' ) ] ] ],
+ insert: [ [ 'r', [ annIndex( 'i' ),
annIndex( 'b' ) ] ] ],
insertedDataOffset: 0,
insertedDataLength: 1
},
@@ -191,14 +191,14 @@
{
type: 'replace',
remove: [
- [ 'b', [ annIndex( 'i', 'foo
bar baz' ) ] ],
- [ 'a', [ annIndex( 'i', 'foo
bar baz' ) ] ],
- [ 'r', [ annIndex( 'i', 'foo
bar baz' ) ] ]
+ [ 'b', [ annIndex( 'i' ) ] ],
+ [ 'a', [ annIndex( 'i' ) ] ],
+ [ 'r', [ annIndex( 'i' ) ] ]
],
insert: [
- [ 'b', [ annIndex( 'i', 'foo
bar baz' ), boldIndex ] ],
- [ 'a', [ annIndex( 'i', 'foo
bar baz' ), boldIndex ] ],
- [ 'r', [ annIndex( 'i', 'foo
bar baz' ), boldIndex ] ]
+ [ 'b', [ annIndex( 'i' ),
boldIndex ] ],
+ [ 'a', [ annIndex( 'i' ),
boldIndex ] ],
+ [ 'r', [ annIndex( 'i' ),
boldIndex ] ]
],
insertedDataOffset: 0,
insertedDataLength: 3
@@ -216,7 +216,7 @@
{
type: 'replace',
remove: [],
- insert: [ [ 'z', [ annIndex( 'i',
'wx<b>y</b>' ), annIndex( 'b', 'y' ) ] ] ],
+ insert: [ [ 'z', [ annIndex( 'i' ),
annIndex( 'b' ) ] ] ],
insertedDataOffset: 0,
insertedDataLength: 1
},
@@ -233,7 +233,7 @@
{
type: 'replace',
remove: [],
- insert: [ [ 'z', [ annIndex( 'i',
'wx<b>y</b>' ) ] ] ],
+ insert: [ [ 'z', [ annIndex( 'i' ) ] ]
],
insertedDataOffset: 0,
insertedDataLength: 1
},
@@ -268,14 +268,14 @@
{
type: 'replace',
remove: [
- [ 'b', [ annIndex( 'i', 'foo
bar<u>baz</u>' ) ] ],
- [ 'a', [ annIndex( 'i', 'foo
bar<u>baz</u>' ) ] ],
- [ 'r', [ annIndex( 'i', 'foo
bar<u>baz</u>' ) ] ]
+ [ 'b', [ annIndex( 'i' ) ] ],
+ [ 'a', [ annIndex( 'i' ) ] ],
+ [ 'r', [ annIndex( 'i' ) ] ]
],
insert: [
- [ 'b', [ annIndex( 'i', 'foo
bar<u>baz</u>' ), annIndex( 'u', 'baz' ), boldIndex ] ],
- [ 'a', [ annIndex( 'i', 'foo
bar<u>baz</u>' ), annIndex( 'u', 'baz' ), boldIndex ] ],
- [ 'r', [ annIndex( 'i', 'foo
bar<u>baz</u>' ), annIndex( 'u', 'baz' ), boldIndex ] ]
+ [ 'b', [ annIndex( 'i' ),
annIndex( 'u' ), boldIndex ] ],
+ [ 'a', [ annIndex( 'i' ),
annIndex( 'u' ), boldIndex ] ],
+ [ 'r', [ annIndex( 'i' ),
annIndex( 'u' ), boldIndex ] ]
],
insertedDataOffset: 0,
insertedDataLength: 3
@@ -313,12 +313,12 @@
{
type: 'replace',
remove: [
- [ 'c', [ annIndex( 'i',
'a<b>bc</b>de' ), annIndex( 'b', 'bc' ) ] ],
- [ 'd', [ annIndex( 'i',
'a<b>bc</b>de' ) ] ]
+ [ 'c', [ annIndex( 'i' ),
annIndex( 'b' ) ] ],
+ [ 'd', [ annIndex( 'i' ) ] ]
],
insert: [
- [ 'c', [ annIndex( 'i',
'a<b>bc</b>de' ), annIndex( 'b', 'bc' ), underlineIndex ] ],
- [ 'd', [ annIndex( 'i',
'a<b>bc</b>de' ), underlineIndex ] ]
+ [ 'c', [ annIndex( 'i' ),
annIndex( 'b' ), underlineIndex ] ],
+ [ 'd', [ annIndex( 'i' ),
underlineIndex ] ]
],
insertedDataOffset: 0,
insertedDataLength: 2
@@ -340,17 +340,17 @@
// then replaces the entire interior.
In real life usage
// there won't usually be two separate
changed regions.
remove: [
- [ 'b', [ annIndex( 'i', 'bar' )
] ],
- [ 'a', [ annIndex( 'i', 'bar' )
] ],
- [ 'r', [ annIndex( 'i', 'bar' )
] ],
+ [ 'b', [ annIndex( 'i' ) ] ],
+ [ 'a', [ annIndex( 'i' ) ] ],
+ [ 'r', [ annIndex( 'i' ) ] ],
' ', 'b', 'a', 'z'
],
// The first insertion get
insert: [
'b', 'a', 'r', ' ',
- [ 'b', [ annIndex( 'b', 'foo' )
] ],
- [ 'a', [ annIndex( 'b', 'foo' )
] ],
- [ 'z', [ annIndex( 'b', 'foo' )
] ]
+ [ 'b', [ annIndex( 'b' ) ] ],
+ [ 'a', [ annIndex( 'b' ) ] ],
+ [ 'z', [ annIndex( 'b' ) ] ]
],
insertedDataOffset: 0,
insertedDataLength: 7
@@ -368,7 +368,7 @@
{
type: 'replace',
remove: [],
- insert: [ 'y', [ 'w', [ annIndex( 'u',
'x' ) ] ] ],
+ insert: [ 'y', [ 'w', [ annIndex( 'u' )
] ] ],
insertedDataOffset: 0,
insertedDataLength: 2
},
diff --git a/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
b/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
index e8aab53..3a3c10f 100644
--- a/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
+++ b/tests/dm/lineardata/ve.dm.ElementLinearData.test.js
@@ -1704,14 +1704,14 @@
data: [
{ type: 'paragraph' },
'F', 'o', 'o', ' ', 'B', 'a', 'r', ' ',
- [ 'B', [ ve.dm.example.annIndex( 'b',
'Baz \nQuux' ) ] ],
- [ 'a', [ ve.dm.example.annIndex( 'b',
'Baz \nQuux' ) ] ],
- [ 'z', [ ve.dm.example.annIndex( 'b',
'Baz \nQuux' ) ] ],
- [ ' ', [ ve.dm.example.annIndex( 'b',
'Baz \nQuux' ) ] ],
- [ 'Q', [ ve.dm.example.annIndex( 'b',
'Baz \nQuux' ) ] ],
- [ 'u', [ ve.dm.example.annIndex( 'b',
'Baz \nQuux' ) ] ],
- [ 'u', [ ve.dm.example.annIndex( 'b',
'Baz \nQuux' ) ] ],
- [ 'x', [ ve.dm.example.annIndex( 'b',
'Baz \nQuux' ) ] ],
+ [ 'B', [ ve.dm.example.annIndex( 'b' )
] ],
+ [ 'a', [ ve.dm.example.annIndex( 'b' )
] ],
+ [ 'z', [ ve.dm.example.annIndex( 'b' )
] ],
+ [ ' ', [ ve.dm.example.annIndex( 'b' )
] ],
+ [ 'Q', [ ve.dm.example.annIndex( 'b' )
] ],
+ [ 'u', [ ve.dm.example.annIndex( 'b' )
] ],
+ [ 'u', [ ve.dm.example.annIndex( 'b' )
] ],
+ [ 'x', [ ve.dm.example.annIndex( 'b' )
] ],
{ type: '/paragraph' },
{ type: 'internalList' },
{ type: '/internalList' }
diff --git a/tests/dm/ve.dm.example.js b/tests/dm/ve.dm.example.js
index f56aa55..1fc7f21 100644
--- a/tests/dm/ve.dm.example.js
+++ b/tests/dm/ve.dm.example.js
@@ -156,14 +156,14 @@
return { type: 'meta/language', attributes: { lang: lang, dir: dir } };
};
-ve.dm.example.annIndex = function ( tagName, text ) {
+ve.dm.example.annIndex = function ( tagName ) {
var ann = ve.copy( {
b: ve.dm.example.bold,
i: ve.dm.example.italic,
u: ve.dm.example.underline
}[ tagName ] );
- ann.originalDomElementsIndex =
ve.dm.IndexValueStore.prototype.indexOfValue( null, '<' + tagName + '>' + text
+ '</' + tagName + '>' );
+ ann.originalDomElementsIndex =
ve.dm.IndexValueStore.prototype.indexOfValue( null, '<' + tagName + '>' + '</'
+ tagName + '>' );
return ve.dm.IndexValueStore.prototype.indexOfValue( ann );
};
@@ -172,10 +172,6 @@
ve.dm.example.italicIndex = 'hefd27ef3bf2041dd';
ve.dm.example.underlineIndex = 'hf214c680fbc361da';
ve.dm.example.strongIndex = 'ha5aaf526d1c3af54';
-
-ve.dm.example.domBoldIndex = 'ha17878c4224059d6';
-ve.dm.example.domItalicIndex = 'h818fb55eaa1f5676';
-ve.dm.example.domUnderlineIndex = 'h6d4db1ae2f34b4b7';
ve.dm.example.inlineSlug = '<span class="ve-ce-branchNode-slug
ve-ce-branchNode-inlineSlug"></span>';
ve.dm.example.blockSlug = '<div class="ve-ce-branchNode-slug
ve-ce-branchNode-blockSlug"></div>';
@@ -1501,6 +1497,33 @@
],
normalizedBody: '<p><i>Foo</i><b>bar</b></p>'
},
+ 'annotation merging': {
+ body: '<p><b>abc</b>X<b>def</b><i>ghi</i></p>',
+ data: [
+ { type: 'paragraph' },
+ [ 'a', [ ve.dm.example.bold ] ],
+ [ 'b', [ ve.dm.example.bold ] ],
+ [ 'c', [ ve.dm.example.bold ] ],
+ 'X',
+ [ 'd', [ ve.dm.example.bold ] ],
+ [ 'e', [ ve.dm.example.bold ] ],
+ [ 'f', [ ve.dm.example.bold ] ],
+ [ 'g', [ ve.dm.example.italic ] ],
+ [ 'h', [ ve.dm.example.italic ] ],
+ [ 'i', [ ve.dm.example.italic ] ],
+ { type: '/paragraph' },
+ { type: 'internalList' },
+ { type: '/internalList' }
+ ],
+ modify: function ( doc ) {
+ doc.commit(
ve.dm.TransactionBuilder.static.newFromRemoval(
+ doc,
+ new ve.Range( 4, 5 )
+ ) );
+ },
+ normalizedBody: '<p><b>abcdef</b><i>ghi</i></p>',
+ fromDataBody: '<p><b>abcdef</b><i>ghi</i></p>'
+ },
'language annotation': {
body: '<p>' +
'<span lang="en">ten</span>' +
@@ -2209,6 +2232,9 @@
body:
'<p><b>Foo</b><b>bar</b><strong>baz</strong></p>' +
'<p><a href="quux">Foo</a><a href="quux">bar</a><a
href="whee">baz</a></p>',
+ normalizedBody:
+ '<p><b>Foobar</b><strong>baz</strong></p>' +
+ '<p><a href="quux">Foobar</a><a
href="whee">baz</a></p>',
data: [
{ type: 'paragraph' },
[ 'F', [ ve.dm.example.bold ] ],
@@ -2239,6 +2265,19 @@
'<p><b>Foobarbaz</b></p>' +
'<p><a href="quux">Foobar</a><a href="whee">baz</a></p>'
},
+ 'adjacent identical annotations with identical content': {
+ body: '<p><b>x</b><b>x</b></p>',
+ normalizedBody: '<p><b>xx</b></p>',
+ data: [
+ { type: 'paragraph' },
+ [ 'x', [ ve.dm.example.bold ] ],
+ [ 'x', [ ve.dm.example.bold ] ],
+ { type: '/paragraph' },
+ { type: 'internalList' },
+ { type: '/internalList' }
+ ],
+ fromDataBody: '<p><b>xx</b></p>'
+ },
'list item with space followed by link': {
body: '<ul><li><p> <a href="Foobar">bar</a></p></li></ul>',
head: '<base href="http://example.com/Foo"; />',
--
To view, visit https://gerrit.wikimedia.org/r/343458
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I1aec6072452051990f1147e4617739620c7b251f
Gerrit-PatchSet: 4
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: DLynch <[email protected]>
Gerrit-Reviewer: Divec <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
