Merge pull request #7840 from ckeditor/i/7838

Fix (utils): `Rect` utility returns wrong sizes in case of a sequenced range. Closes #7838.

Feature (utils): Introduced the `Rect#getBoundingRect()` method that returns a `Rect` instance containing all other rectangles. Closes #7858.

packages/ckeditor5-table/src/utils/ui/contextualballoon.js

@@ -87,11 +87,7 @@ export function getBalloonCellPositionData( editor ) {
 
 	if ( selection.rangeCount > 1 ) {
 		return {
-			target: () => createBoundingRect( selection.getRanges(), modelRange => {
-				const modelTableCell = getTableCellAtPosition( modelRange.start );
-				const viewTableCell = mapper.toViewElement( modelTableCell );
-				return new Rect( domConverter.viewToDom( viewTableCell ) );
-			} ),
+			target: () => createBoundingRect( selection.getRanges(), editor ),
 			positions: BALLOON_POSITIONS
 		};
 	}
@@ -115,30 +111,19 @@ function getTableCellAtPosition( position ) {
 	return isTableCellSelected ? position.nodeAfter : position.findAncestor( 'tableCell' );
 }
 
-// Returns bounding rect for list of rects.
+// Returns bounding rectangle for given model ranges.
 //
-// @param {Array.<module:utils/dom/rect~Rect>|Array.<*>} list List of `Rect`s or any list to map by `mapFn`.
-// @param {Function} mapFn Mapping function for list elements.
+// @param {Iterable.<module:engine/model/range~Range>} ranges Model ranges that the bounding rect should be returned for.
+// @param {module:core/editor/editor~Editor} editor The editor instance.
 // @returns {module:utils/dom/rect~Rect}
-function createBoundingRect( list, mapFn ) {
-	const rectData = {
-		left: Number.POSITIVE_INFINITY,
-		top: Number.POSITIVE_INFINITY,
-		right: Number.NEGATIVE_INFINITY,
-		bottom: Number.NEGATIVE_INFINITY
-	};
-
-	for ( const item of list ) {
-		const rect = mapFn( item );
-
-		rectData.left = Math.min( rectData.left, rect.left );
-		rectData.top = Math.min( rectData.top, rect.top );
-		rectData.right = Math.max( rectData.right, rect.right );
-		rectData.bottom = Math.max( rectData.bottom, rect.bottom );
-	}
-
-	rectData.width = rectData.right - rectData.left;
-	rectData.height = rectData.bottom - rectData.top;
+function createBoundingRect( ranges, editor ) {
+	const mapper = editor.editing.mapper;
+	const domConverter = editor.editing.view.domConverter;
+	const rects = Array.from( ranges ).map( range => {
+		const modelTableCell = getTableCellAtPosition( range.start );
+		const viewTableCell = mapper.toViewElement( modelTableCell );
+		return new Rect( domConverter.viewToDom( viewTableCell ) );
+	} );
 
-	return new Rect( rectData );
+	return Rect.getBoundingRect( rects );
 }

packages/ckeditor5-utils/src/dom/rect.js

@@ -78,7 +78,8 @@ export default class Rect {
 			// @if CK_DEBUG // }
 
 			if ( isSourceRange ) {
-				copyRectProperties( this, Rect.getDomRangeRects( source )[ 0 ] );
+				const rangeRects = Rect.getDomRangeRects( source );
+				copyRectProperties( this, Rect.getBoundingRect( rangeRects ) );
 			} else {
 				copyRectProperties( this, source.getBoundingClientRect() );
 			}
@@ -381,6 +382,40 @@ export default class Rect {
 
 		return rects;
 	}
+
+	/**
+	 * Returns a bounding rectangle that contains all the given `rects`.
+	 *
+	 * @param {Iterable.<module:utils/dom/rect~Rect>} rects A list of rectangles that should be contained in the result rectangle.
+	 * @returns {module:utils/dom/rect~Rect|null} Bounding rectangle or `null` if no `rects` were given.
+	 */
+	static getBoundingRect( rects ) {
+		const boundingRectData = {
+			left: Number.POSITIVE_INFINITY,
+			top: Number.POSITIVE_INFINITY,
+			right: Number.NEGATIVE_INFINITY,
+			bottom: Number.NEGATIVE_INFINITY
+		};
+		let rectangleCount = 0;
+
+		for ( const rect of rects ) {
+			rectangleCount++;
+
+			boundingRectData.left = Math.min( boundingRectData.left, rect.left );
+			boundingRectData.top = Math.min( boundingRectData.top, rect.top );
+			boundingRectData.right = Math.max( boundingRectData.right, rect.right );
+			boundingRectData.bottom = Math.max( boundingRectData.bottom, rect.bottom );
+		}
+
+		if ( rectangleCount == 0 ) {
+			return null;
+		}
+
+		boundingRectData.width = boundingRectData.right - boundingRectData.left;
+		boundingRectData.height = boundingRectData.bottom - boundingRectData.top;
+
+		return new Rect( boundingRectData );
+	}
 }
 
 // Acquires all the rect properties from the passed source.

packages/ckeditor5-utils/tests/dom/rect.js

@@ -52,6 +52,45 @@ describe( 'Rect', () => {
 			assertRect( new Rect( range ), geometry );
 		} );
 
+		it( 'should accept Range (non–collapsed, sequenced horizontally)', () => {
+			const firstGeometry = geometry;
+			const secondGeometry = Object.assign( {}, geometry, {
+				right: 50,
+				left: 40,
+				width: 10
+			} );
+
+			const range = document.createRange();
+			range.selectNode( document.body );
+			sinon.stub( range, 'getClientRects' ).returns( [ firstGeometry, secondGeometry ] );
+
+			const expectedGeometry = Object.assign( {}, geometry, {
+				width: 30,
+				right: 50
+			} );
+
+			assertRect( new Rect( range ), expectedGeometry );
+		} );
+
+		it( 'should accept Range (non–collapsed, sequenced vertically)', () => {
+			const firstGeometry = geometry;
+			const secondGeometry = Object.assign( {}, geometry, {
+				top: 30,
+				bottom: 40
+			} );
+
+			const range = document.createRange();
+			range.selectNode( document.body );
+			sinon.stub( range, 'getClientRects' ).returns( [ firstGeometry, secondGeometry ] );
+
+			const expectedGeometry = Object.assign( {}, geometry, {
+				height: 30,
+				bottom: 40
+			} );
+
+			assertRect( new Rect( range ), expectedGeometry );
+		} );
+
 		// https://github.com/ckeditor/ckeditor5-utils/issues/153
 		it( 'should accept Range (collapsed)', () => {
 			const range = document.createRange();
@@ -1053,6 +1092,53 @@ describe( 'Rect', () => {
 			assertRect( rects[ 0 ], expectedGeometry );
 		} );
 	} );
+
+	describe( 'getBoundingRect()', () => {
+		it( 'should not return a rect instance when no rectangles were given', () => {
+			expect( Rect.getBoundingRect( [] ) ).to.be.null;
+		} );
+
+		it( 'should calculate proper rectangle when multiple rectangles were given', () => {
+			const rects = [
+				new Rect( geometry ),
+				new Rect( {
+					top: 10,
+					right: 100,
+					bottom: 20,
+					left: 80,
+					width: 20,
+					height: 10
+				} ),
+				new Rect( {
+					top: 50,
+					right: 50,
+					bottom: 60,
+					left: 30,
+					width: 20,
+					height: 10
+				} )
+			];
+
+			assertRect( Rect.getBoundingRect( rects ), {
+				top: 10,
+				right: 100,
+				bottom: 60,
+				left: 20,
+				width: 80,
+				height: 50
+			} );
+		} );
+
+		it( 'should calculate proper rectangle when a single rectangles was given', () => {
+			const rectangles = new Set( [ new Rect( geometry ) ] );
+			assertRect( Rect.getBoundingRect( rectangles ), geometry );
+		} );
+
+		it( 'should return proper type', () => {
+			const rectangles = new Set( [ new Rect( geometry ) ] );
+			expect( Rect.getBoundingRect( rectangles ) ).to.be.instanceOf( Rect );
+		} );
+	} );
 } );
 
 function assertRect( rect, expected ) {