Other: Added more cases of affected markers on merging in writer.

ckeditor · May 19, 2019 · 90b04fc · 90b04fc
1 parent 8c39bdb
commit 90b04fc
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 19 deletions.
diff --git a/src/controller/datacontroller.js b/src/controller/datacontroller.js
@@ -297,6 +297,7 @@ export default class DataController {
 	 */
 	set( data ) {
 		let newData = {};
+
 		if ( typeof data === 'string' ) {
 			newData.main = data; // Default root is 'main'. To set data on a different root, object should be passed.
 		} else {

diff --git a/src/model/writer.js b/src/model/writer.js
@@ -1349,10 +1349,31 @@ export default class Writer {
 				const elementBefore = positionOrRange.nodeBefore;
 				const elementAfter = positionOrRange.nodeAfter;
 
-				const affectedOnLeft = markerRange.start.parent == elementBefore && markerRange.start.isAtEnd;
-				const affectedOnRight = markerRange.end.parent == elementAfter && markerRange.end.offset == 0;
-
-				isAffected = affectedOnLeft || affectedOnRight;
+				//               Start:  <p>Foo[</p><p>Bar]</p>
+				//         After merge:  <p>Foo[Bar]</p>
+				// After undoing split:  <p>Foo</p><p>[Bar]</p>     <-- incorrect, needs remembering for undo.
+				//
+				const affectedInLeftElement = markerRange.start.parent == elementBefore && markerRange.start.isAtEnd;
+
+				//               Start:  <p>[Foo</p><p>]Bar</p>
+				//         After merge:  <p>[Foo]Bar</p>
+				// After undoing split:  <p>[Foo]</p><p>Bar</p>     <-- incorrect, needs remembering for undo.
+				//
+				const affectedInRightElement = markerRange.end.parent == elementAfter && markerRange.end.offset == 0;
+
+				//               Start:  <p>[Foo</p>]<p>Bar</p>
+				//         After merge:  <p>[Foo]Bar</p>
+				// After undoing split:  <p>[Foo]</p><p>Bar</p>     <-- incorrect, needs remembering for undo.
+				//
+				const affectedAfterLeftElement = markerRange.end.nodeAfter == elementAfter;
+
+				//               Start:  <p>Foo</p>[<p>Bar]</p>
+				//         After merge:  <p>Foo[Bar]</p>
+				// After undoing split:  <p>Foo</p><p>[Bar]</p>     <-- incorrect, needs remembering for undo.
+				//
+				const affectedBeforeRightElement = markerRange.start.nodeAfter == elementAfter;
+
+				isAffected = affectedInLeftElement || affectedInRightElement || affectedAfterLeftElement || affectedBeforeRightElement;
 			}
 
 			if ( isAffected ) {

diff --git a/tests/model/writer.js b/tests/model/writer.js
@@ -1425,29 +1425,50 @@ describe( 'Writer', () => {
 			expect( docFrag.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foobar' );
 		} );
 
-		it( 'should create a marker operation if a marker was affected', () => {
-			const markerRange = new Range( Position._createAt( p2, 0 ), Position._createAt( p2, 0 ) );
+		describe( 'should create a marker operation if a marker was affected', () => {
+			it( '<p>Foo[</p><p>Bar]</p>', () => {
+				test( p1, 'end', p2, 0 );
+			} );
 
-			addMarker( 'name', {
-				range: markerRange,
-				usingOperation: true
+			it( '<p>[Foo</p><p>]Bar</p>', () => {
+				test( p1, 0, p2, 0 );
 			} );
 
-			const documentVersion = model.document.version;
+			it( '<p>[Foo</p>]<p>Bar</p>', () => {
+				test( p1, 0, root, 1 );
+			} );
 
-			merge( Position._createAfter( p1 ) );
+			it( '<p>Foo</p>[<p>Bar]</p>', () => {
+				test( root, 1, p2, 'end' );
+			} );
 
-			const history = model.document.history;
+			function test( startElement, startOffset, endElement, endOffset ) {
+				const markerRange = new Range(
+					Position._createAt( startElement, startOffset ),
+					Position._createAt( endElement, endOffset )
+				);
 
-			const lastOperation = history._operations[ history._operations.length - 1 ];
-			const secondLastOperation = history._operations[ history._operations.length - 2 ];
+				addMarker( 'name', {
+					range: markerRange,
+					usingOperation: true
+				} );
 
-			expect( secondLastOperation.type ).to.equal( 'marker' );
-			expect( secondLastOperation.oldRange.isEqual( markerRange ) );
-			expect( secondLastOperation.newRange.isEqual( markerRange ) );
+				const documentVersion = model.document.version;
 
-			expect( lastOperation.type ).to.equal( 'merge' );
-			expect( model.document.version ).to.equal( documentVersion + 2 );
+				merge( Position._createAfter( p1 ) );
+
+				const history = model.document.history;
+
+				const lastOperation = history._operations[ history._operations.length - 1 ];
+				const secondLastOperation = history._operations[ history._operations.length - 2 ];
+
+				expect( secondLastOperation.type ).to.equal( 'marker' );
+				expect( secondLastOperation.oldRange.isEqual( markerRange ) );
+				expect( secondLastOperation.newRange.isEqual( markerRange ) );
+
+				expect( lastOperation.type ).to.equal( 'merge' );
+				expect( model.document.version ).to.equal( documentVersion + 2 );
+			}
 		} );
 
 		it( 'should not create a marker operation if affected marker was not using operations', () => {