Browse Source

Added: Support for multiple ranges is selection postfixer for isLimit=true elements.

Maciej Gołaszewski 7 years ago
parent
commit
3b940ee757

+ 48 - 3
packages/ckeditor5-engine/src/controller/editingcontroller.js

@@ -100,7 +100,9 @@ export default class EditingController {
 		this.downcastDispatcher.on( 'selection', convertCollapsedSelection(), { priority: 'low' } );
 
 		// Add selection post fixer.
-		doc.registerPostFixer( writer => {
+		doc.registerPostFixer( selectionPostFixer );
+
+		function selectionPostFixer( writer ) {
 			const ranges = [];
 
 			let needsUpdate = false;
@@ -116,10 +118,15 @@ export default class EditingController {
 				}
 			}
 
+			// If any of ranges were corrected update the selection.
 			if ( needsUpdate ) {
-				writer.setSelection( ranges, { backward: selection.isBackward } );
+				// The above algorithm might create ranges that intersects each other when selection contains more then one range.
+				// This is case happens mostly on Firefox which creates multiple ranges for selected table.
+				const safeRange = combineRanges( ranges );
+
+				writer.setSelection( safeRange, { backward: selection.isBackward } );
 			}
-		} );
+		}
 
 		// Binds {@link module:engine/view/document~Document#roots view roots collection} to
 		// {@link module:engine/model/document~Document#roots model roots collection} so creating
@@ -214,3 +221,41 @@ function fixSelectionOnLimitBlock( schema, fixedPosition ) {
 
 	return new Range( fixedPosition );
 }
+
+function combineRanges( ranges ) {
+	const combined = [];
+
+	let previousRange;
+
+	for ( let i = 0; i < ranges.length; i++ ) {
+		const range = ranges[ i ];
+
+		if ( !previousRange ) {
+			previousRange = range;
+			combined.push( previousRange );
+			continue;
+		}
+
+		// Do not push same ranges (ie might be created in a table)
+		if ( range.isEqual( previousRange ) ) {
+			continue;
+		}
+
+		if ( range.isIntersecting( previousRange ) ) {
+			const newStart = previousRange.start.isBefore( range.start ) ? previousRange.start : range.start;
+			const newEnd = range.end.isAfter( previousRange.end ) ? range.end : previousRange.end;
+			const newRange = new Range( newStart, newEnd );
+
+			combined.splice( combined.indexOf( previousRange ), 1, newRange );
+
+			previousRange = newRange;
+
+			continue;
+		}
+
+		previousRange = range;
+		combined.push( range );
+	}
+
+	return combined;
+}

+ 95 - 0
packages/ckeditor5-engine/tests/controller/editingcontroller.js

@@ -529,6 +529,97 @@ describe( 'EditingController', () => {
 						'<p>ba}z</p>'
 					);
 				} );
+
+				it( 'should fix multiple ranges #1', () => {
+					model.change( writer => {
+						const ranges = [
+							new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 1, 0 ] ) ),
+							new ModelRange( new ModelPosition( modelRoot, [ 1, 0, 0, 0 ] ), new ModelPosition( modelRoot, [ 1, 1 ] ) )
+						];
+						writer.setSelection( ranges );
+					} );
+
+					expect( getViewData( editing.view ) ).to.equal(
+						'<p>f{oo</p>' +
+						'<table><tr><td>aaa</td><td>bbb</td></tr></table>]' +
+						'<p>bar</p>'
+					);
+				} );
+
+				it( 'should fix multiple ranges #2', () => {
+					model.change( writer => {
+						const ranges = [
+							new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 1, 0 ] ) ),
+							new ModelRange( new ModelPosition( modelRoot, [ 1, 0, 0, 0 ] ), new ModelPosition( modelRoot, [ 2, 2 ] ) )
+						];
+
+						writer.setSelection( ranges );
+					} );
+
+					expect( getViewData( editing.view ) ).to.equal(
+						'<p>f{oo</p>' +
+						'<table><tr><td>aaa</td><td>bbb</td></tr></table>' +
+						'<p>ba}r</p>'
+					);
+				} );
+
+				it( 'should fix multiple ranges #3', () => {
+					modelSetData( model, '' );
+
+					const modelData = new ModelDocumentFragment( parse(
+						'<paragraph>foo</paragraph>' +
+						'<table>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'<tableRow><tableCell>aaa</tableCell><tableCell>bbb</tableCell></tableRow>' +
+						'</table>' +
+						'<paragraph>baz</paragraph>',
+						model.schema
+					)._children );
+
+					model.change( writer => {
+						writer.insert( modelData, model.document.getRoot() );
+
+						const ranges = [
+							new ModelRange( new ModelPosition( modelRoot, [ 1, 0, 0, 0 ] ), new ModelPosition( modelRoot, [ 1, 1 ] ) ),
+							new ModelRange( new ModelPosition( modelRoot, [ 1, 1, 0, 0 ] ), new ModelPosition( modelRoot, [ 1, 2 ] ) ),
+							new ModelRange( new ModelPosition( modelRoot, [ 1, 2, 0, 0 ] ), new ModelPosition( modelRoot, [ 1, 3 ] ) ),
+							new ModelRange( new ModelPosition( modelRoot, [ 1, 3, 0, 0 ] ), new ModelPosition( modelRoot, [ 2, 1 ] ) )
+						];
+
+						writer.setSelection( ranges );
+					} );
+
+					expect( getViewData( editing.view ) ).to.equal(
+						'<p>foo</p>' +
+						'[<table>' +
+						'<tr><td>aaa</td><td>bbb</td></tr>' +
+						'<tr><td>aaa</td><td>bbb</td></tr>' +
+						'<tr><td>aaa</td><td>bbb</td></tr>' +
+						'<tr><td>aaa</td><td>bbb</td></tr>' +
+						'</table>' +
+						'<p>b}az</p>'
+					);
+				} );
+
+				it( 'should fix multiple ranges #4', () => {
+					model.change( writer => {
+						const ranges = [
+							new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 1, 0 ] ) ),
+							new ModelRange( new ModelPosition( modelRoot, [ 1, 0, 0, 0 ] ), new ModelPosition( modelRoot, [ 2, 1 ] ) ),
+							new ModelRange( new ModelPosition( modelRoot, [ 2, 2 ] ), new ModelPosition( modelRoot, [ 2, 3 ] ) )
+						];
+
+						writer.setSelection( ranges );
+					} );
+
+					expect( getViewData( editing.view ) ).to.equal(
+						'<p>f{oo</p>' +
+						'<table><tr><td>aaa</td><td>bbb</td></tr></table>' +
+						'<p>b}a{r}</p>'
+					);
+				} );
 			} );
 
 			describe( 'collapsed selection', () => {
@@ -544,6 +635,10 @@ describe( 'EditingController', () => {
 					);
 				} );
 			} );
+
+			afterEach( () => {
+				editing.destroy();
+			} );
 		} );
 	} );