8
0
Просмотр исходного кода

Changes in engine.view.writer: renamed breakAttributes and mergeAttributes method, getParentAttribute method is now private, fixed ranges and positions container validation. Updated docs and tests.

Szymon Kupś 9 лет назад
Родитель
Сommit
3bb6e97bf6

+ 1 - 1
packages/ckeditor5-engine/src/conversion/model-converter-builder.js

@@ -127,7 +127,7 @@ class ModelConverterBuilder {
 	 * Default priority is `10`.
 	 *
 	 * **Note:** Keep in mind that event priority, that is set by this modifier, is used for attribute priority
-	 * when {@link engine.view.Writer} is used. This changes how model attributes converter to view elements are
+	 * when {@link engine.view.writer} is used. This changes how model attributes converter to view elements are
 	 * ordered, i.e.: `<strong><em>foo</em></strong>` vs `<em><strong>foo</strong></em>`. Using priority you can also
 	 * prevent node merging, i.e.: `<span class="bold"><span class="theme">foo</span><span>` vs `<span class="bold theme">foo</span>`.
 	 * If you want to prevent merging, just set different priority for both converters.

+ 3 - 3
packages/ckeditor5-engine/src/conversion/model-selection-to-view-converters.js

@@ -86,7 +86,7 @@ export function convertCollapsedSelection() {
 
 		const modelPosition = selection.getFirstPosition();
 		const viewPosition = conversionApi.mapper.toViewPosition( modelPosition );
-		const brokenPosition = viewWriter.breakAttributes( viewPosition );
+		const brokenPosition = viewWriter.breakAt( viewPosition );
 
 		conversionApi.viewSelection.removeAllRanges();
 		conversionApi.viewSelection.addRange( new ViewRange( brokenPosition, brokenPosition ) );
@@ -201,9 +201,9 @@ export function clearAttributes() {
 		for ( let range of conversionApi.viewSelection.getRanges() ) {
 			// Not collapsed selection should not have artifacts.
 			if ( range.isCollapsed ) {
-				// Position might be in the node removed by the Writer.
+				// Position might be in the node removed by the view writer.
 				if ( range.end.parent.getDocument() ) {
-					viewWriter.mergeAttributes( range.start );
+					viewWriter.mergeAt( range.start );
 				}
 			}
 		}

+ 1 - 1
packages/ckeditor5-engine/src/view/attributeelement.js

@@ -12,7 +12,7 @@ const DEFAULT_PRIORITY = 10;
 
 /**
  * Attributes are elements which define document presentation. They are mostly elements like `<b>` or `<span>`.
- * Attributes can be broken and merged by the {@link engine.view.Writer}.
+ * Attributes can be broken and merged by the {@link engine.view.writer writer}.
  *
  * Editing engine does not define fixed HTML DTD. This is why the type of the {@link engine.view.Element} need to
  * be defined by the feature developer. Creating an element you should use {@link engine.view.ContainerElement}

+ 3 - 3
packages/ckeditor5-engine/src/view/containerelement.js

@@ -21,15 +21,15 @@ import Element from './element.js';
  * DOM properly. {@link engine.view.DomConverter} will ensure that `ContainerElement` is editable and it is possible
  * to put caret inside it, even if the container is empty.
  *
- * Secondly, {@link engine.view.Writer} uses this information.
- * Nodes {@link engine.view.Writer#breakAttributes breaking} and {@link engine.view.Writer#mergeAttributes merging}
+ * Secondly, {@link engine.view.writer writer} uses this information.
+ * Nodes {@link engine.view.writer.breakAt breaking} and {@link engine.view.writer#mergeAt merging}
  * is performed only in a bounds of a container nodes.
  *
  * For instance if `<p>` is an container and `<b>` is attribute:
  *
  *		<p><b>fo^o</b></p>
  *
- * {@link engine.view.Writer#breakAttributes breakAttributes} will create:
+ * {@link engine.view.writer.breakAt breakAt} will create:
  *
  *		<p><b>fo</b><b>o</b></p>
  *

+ 90 - 108
packages/ckeditor5-engine/src/view/writer.js

@@ -21,10 +21,9 @@ import isIterable from '../../utils/isiterable.js';
  */
 
 export default {
-	getParentContainer,
-	breakAttributes,
+	breakAt,
 	breakRange,
-	mergeAttributes,
+	mergeAt,
 	insert,
 	remove,
 	move,
@@ -33,30 +32,6 @@ export default {
 	unwrap
 };
 
-/**
- * Returns first parent container of specified {@link engine.view.Position Position}.
- * Position's parent node is checked as first, then next parents are checked.
- *
- * Note that {@link engine.view.DocumentFragment DocumentFragment} is thread like a container.
- *
- * @function engine.view.writer.getParentContainer
- * @param {engine.view.Position} position Position used as a start point to locate parent container.
- * @returns {engine.view.ContainerElement|engine.view.DocumentFragment|undefined} Parent container element or
- * `undefined` if container is not found.
- */
-export function getParentContainer( position ) {
-	let parent = position.parent;
-
-	while ( !isContainerOrFragment( parent ) ) {
-		if ( !parent ) {
-			return undefined;
-		}
-		parent = parent.parent;
-	}
-
-	return parent;
-}
-
 /**
  * Breaks attribute nodes at provided position. It breaks `attribute` nodes inside `container` node.
  *
@@ -66,20 +41,20 @@ export function getParentContainer( position ) {
  *		<p>foo<b><u>{}bar</u></b></p> -> <p>foo{}<b><u>bar</u></b></p>
  *		<p>foo<b><u>b{}ar</u></b></p> -> <p>foo<b><u>b</u></b>[]<b><u>ar</u></b></p>
  *
- * Note that {@link engine.view.DocumentFragment DocumentFragment} is thread like a container.
+ * Note that {@link engine.view.DocumentFragment DocumentFragment} is treated like a container.
  *
  * @see engine.view.AttributeElement
  * @see engine.view.ContainerElement
- * @function engine.view.writer.breakAttributes
+ * @function engine.view.writer.breakAt
  * @param {engine.view.Position} position Position where to break attributes.
  * @returns {engine.view.Position} New position after breaking the attributes.
  */
-export function breakAttributes( position ) {
-	return _breakAttributes( position, false );
+export function breakAt( position ) {
+	return _breakAt( position, false );
 }
 
 /**
- * Uses {@link engine.view.writer.breakAttributes breakAttributes} method to break attributes on
+ * Uses {@link engine.view.writer.breakAt breakAt} method to break attributes on
  * {@link engine.view.Range#start start} and {@link engine.view.Range#end end} positions of
  * provided {@link engine.view.Range Range}.
  *
@@ -87,12 +62,12 @@ export function breakAttributes( position ) {
  * {@link engine.view.Range#start start} and {@link engine.view.Range#end end} positions are not placed inside
  * same parent container.
  *
- * Note that {@link engine.view.DocumentFragment DocumentFragment} is thread like a container.
+ * Note that {@link engine.view.DocumentFragment DocumentFragment} is treated like a container.
  *
- * @see engine.view.writer.breakAttributes
+ * @see engine.view.writer.breakAt
  * @function engine.view.writer.breakRange
  * @param {engine.view.Range} range Range which `start` and `end` positions will be used to break attributes.
- * @returns {engine.view.Range} New range with located at break positions.
+ * @returns {engine.view.Range} New range with boundaries located at break positions.
  */
 export function breakRange( range ) {
 	return _breakRange( range );
@@ -115,11 +90,11 @@ export function breakRange( range ) {
  *
  * @see engine.view.AttributeElement
  * @see engine.view.ContainerElement
- * @function engine.view.writer.mergeAttributes
+ * @function engine.view.writer.mergeAt
  * @param {engine.view.Position} position Merge position.
  * @returns {engine.view.Position} Position after merge.
  */
-export function mergeAttributes( position ) {
+export function mergeAt( position ) {
 	const positionOffset = position.offset;
 	const positionParent = position.parent;
 
@@ -134,7 +109,7 @@ export function mergeAttributes( position ) {
 		const offset = positionParent.getIndex();
 		positionParent.remove();
 
-		return mergeAttributes( new Position( parent, offset ) );
+		return mergeAt( new Position( parent, offset ) );
 	}
 
 	const nodeBefore = positionParent.getChild( positionOffset - 1 );
@@ -164,7 +139,7 @@ export function mergeAttributes( position ) {
 
 		// New position is located inside the first node, before new nodes.
 		// Call this method recursively to merge again if needed.
-		return mergeAttributes( new Position( nodeBefore, count ) );
+		return mergeAt( new Position( nodeBefore, count ) );
 	}
 
 	return position;
@@ -193,11 +168,21 @@ export function insert( position, nodes ) {
 	validateNodesToInsert( nodes );
 
 	const container = getParentContainer( position );
-	const insertionPosition = _breakAttributes( position, true );
+
+	if ( !container ) {
+		/**
+		 * Position container cannot be found.
+		 *
+		 * @error view-writer-invalid-position-container
+		 */
+		throw new CKEditorError( 'view-writer-invalid-position-container' );
+	}
+
+	const insertionPosition = _breakAt( position, true );
 
 	const length = container.insertChildren( insertionPosition.offset, nodes );
 	const endPosition = insertionPosition.getShiftedBy( length );
-	const start = mergeAttributes( insertionPosition );
+	const start = mergeAt( insertionPosition );
 
 	// When no nodes were inserted - return collapsed range.
 	if ( length === 0 ) {
@@ -208,7 +193,7 @@ export function insert( position, nodes ) {
 			endPosition.offset--;
 		}
 
-		const end = mergeAttributes( endPosition );
+		const end = mergeAt( endPosition );
 
 		return new Range( start, end );
 	}
@@ -227,15 +212,7 @@ export function insert( position, nodes ) {
  * @returns {engine.view.DocumentFragment} Document fragment containing removed nodes.
  */
 export function remove( range ) {
-	// Range should be placed inside one container.
-	if ( getParentContainer( range.start ) !== getParentContainer( range.end ) ) {
-		/**
-		 * Range is not placed inside same container.
-		 *
-		 * @error view-writer-invalid-range-container
-		 */
-		throw new CKEditorError( 'view-writer-invalid-range-container' );
-	}
+	validateRangeContainer( range );
 
 	// If range is collapsed - nothing to remove.
 	if ( range.isCollapsed ) {
@@ -252,7 +229,7 @@ export function remove( range ) {
 	const removed = parentContainer.removeChildren( breakStart.offset, count );
 
 	// Merge after removing.
-	const mergePosition = mergeAttributes( breakStart );
+	const mergePosition = mergeAt( breakStart );
 	range.start = mergePosition;
 	range.end = Position.createFromPosition( mergePosition );
 
@@ -301,15 +278,7 @@ export function wrap( range, attribute ) {
 		throw new CKEditorError( 'view-writer-wrap-invalid-attribute' );
 	}
 
-	// Range should be placed inside one container.
-	if ( getParentContainer( range.start ) !== getParentContainer( range.end ) ) {
-		/**
-		 * Range is not placed inside same container.
-		 *
-		 * @error view-writer-invalid-range-container
-		 */
-		throw new CKEditorError( 'view-writer-invalid-range-container' );
-	}
+	validateRangeContainer( range );
 
 	// If range is collapsed - nothing to wrap.
 	if ( range.isCollapsed ) {
@@ -344,13 +313,13 @@ export function wrap( range, attribute ) {
 	const newRange = wrapChildren( parentContainer, unwrappedRange.start.offset, unwrappedRange.end.offset, attribute );
 
 	// Merge attributes at the both ends and return a new range.
-	const start = mergeAttributes( newRange.start );
+	const start = mergeAt( newRange.start );
 
 	// If start position was merged - move end position back.
 	if ( !start.isEqual( newRange.start ) ) {
 		newRange.end.offset--;
 	}
-	const end = mergeAttributes( newRange.end );
+	const end = mergeAt( newRange.end );
 
 	return new Range( start, end );
 }
@@ -438,15 +407,7 @@ export function unwrap( range, attribute ) {
 		throw new CKEditorError( 'view-writer-unwrap-invalid-attribute' );
 	}
 
-	// Range should be placed inside one container.
-	if ( getParentContainer( range.start ) !== getParentContainer( range.end ) ) {
-		/**
-		 * Range is not placed inside same container.
-		 *
-		 * @error view-writer-invalid-range-container
-		 */
-		throw new CKEditorError( 'view-writer-invalid-range-container' );
-	}
+	validateRangeContainer( range );
 
 	// If range is collapsed - nothing to unwrap.
 	if ( range.isCollapsed ) {
@@ -473,21 +434,40 @@ export function unwrap( range, attribute ) {
 	const newRange = unwrapChildren( parentContainer, breakStart.offset, breakEnd.offset, attribute );
 
 	// Merge attributes at the both ends and return a new range.
-	const start = mergeAttributes( newRange.start );
+	const start = mergeAt( newRange.start );
 
 	// If start position was merged - move end position back.
 	if ( !start.isEqual( newRange.start ) ) {
 		newRange.end.offset--;
 	}
-	const end = mergeAttributes( newRange.end );
+	const end = mergeAt( newRange.end );
 
 	return new Range( start, end );
 }
 
+// Returns first parent container of specified {@link engine.view.Position Position}.
+// Position's parent node is checked as first, then next parents are checked.
+// Note that {@link engine.view.DocumentFragment DocumentFragment} is treated like a container.
+//
+// @param {engine.view.Position} position Position used as a start point to locate parent container.
+// @returns {engine.view.ContainerElement|engine.view.DocumentFragment|undefined} Parent container element or
+// `undefined` if container is not found.
+function getParentContainer( position ) {
+	let parent = position.parent;
+
+	while ( !isContainerOrFragment( parent ) ) {
+		if ( !parent ) {
+			return undefined;
+		}
+		parent = parent.parent;
+	}
+
+	return parent;
+}
+
 // Function used by both public breakRange (without splitting text nodes) and by other methods (with
 // splitting text nodes).
 //
-// @private
 // @param {engine.view.Range} range Range which `start` and `end` positions will be used to break attributes.
 // @param {Boolean} [forceSplitText = false] If set to `true`, will break text nodes even if they are directly in
 // container element. This behavior will result in incorrect view state, but is needed by other view writing methods
@@ -497,26 +477,18 @@ function _breakRange( range, forceSplitText = false ) {
 	const rangeStart = range.start;
 	const rangeEnd = range.end;
 
-	// Range should be placed inside one container.
-	if ( getParentContainer( rangeStart ) !== getParentContainer( rangeEnd ) ) {
-		/**
-		 * Range is not placed inside same container.
-		 *
-		 * @error view-writer-invalid-range-container
-		 */
-		throw new CKEditorError( 'view-writer-invalid-range-container' );
-	}
+	validateRangeContainer( range );
 
 	// Break at the collapsed position. Return new collapsed range.
 	if ( range.isCollapsed ) {
-		const position = _breakAttributes( range.start, forceSplitText );
+		const position = _breakAt( range.start, forceSplitText );
 
 		return new Range( position, position );
 	}
 
-	const breakEnd = _breakAttributes( rangeEnd, forceSplitText );
+	const breakEnd = _breakAt( rangeEnd, forceSplitText );
 	const count = breakEnd.parent.getChildCount();
-	const breakStart = _breakAttributes( rangeStart, forceSplitText );
+	const breakStart = _breakAt( rangeStart, forceSplitText );
 
 	// Calculate new break end offset.
 	breakEnd.offset += breakEnd.parent.getChildCount() - count;
@@ -524,16 +496,15 @@ function _breakRange( range, forceSplitText = false ) {
 	return new Range( breakStart, breakEnd );
 }
 
-// Function used by both public breakAttributes (without splitting text nodes) and by other methods (with
+// Function used by public breakAt (without splitting text nodes) and by other methods (with
 // splitting text nodes).
 //
-// @private
 // @param {engine.view.Position} position Position where to break attributes.
 // @param {Boolean} [forceSplitText = false] If set to `true`, will break text nodes even if they are directly in
 // container element. This behavior will result in incorrect view state, but is needed by other view writing methods
 // which then fixes view state. Defaults to `false`.
 // @returns {engine.view.Position} New position after breaking the attributes.
-function _breakAttributes( position, forceSplitText = false ) {
+function _breakAt( position, forceSplitText = false ) {
 	const positionOffset = position.offset;
 	const positionParent = position.parent;
 
@@ -549,7 +520,7 @@ function _breakAttributes( position, forceSplitText = false ) {
 
 	// Break text and start again in new position.
 	if ( positionParent instanceof Text ) {
-		return _breakAttributes( breakTextNode( position ), forceSplitText );
+		return _breakAt( breakTextNode( position ), forceSplitText );
 	}
 
 	const length = positionParent.getChildCount();
@@ -560,7 +531,7 @@ function _breakAttributes( position, forceSplitText = false ) {
 	if ( positionOffset == length ) {
 		const newPosition = new Position( positionParent.parent, positionParent.getIndex() + 1 );
 
-		return _breakAttributes( newPosition, forceSplitText );
+		return _breakAt( newPosition, forceSplitText );
 	} else
 	// <p>foo<b><u>{}bar</u></b></p>
 	// <p>foo<b>[]<u>bar</u></b></p>
@@ -568,7 +539,7 @@ function _breakAttributes( position, forceSplitText = false ) {
 	if ( positionOffset === 0 ) {
 		const newPosition = new Position( positionParent.parent, positionParent.getIndex() );
 
-		return _breakAttributes( newPosition, forceSplitText );
+		return _breakAt( newPosition, forceSplitText );
 	}
 	// <p>foo<b><u>b{}ar</u></b></p>
 	// <p>foo<b><u>b[]ar</u></b></p>
@@ -593,14 +564,13 @@ function _breakAttributes( position, forceSplitText = false ) {
 		// Create new position to work on.
 		const newPosition = new Position( positionParent.parent, offsetAfter );
 
-		return _breakAttributes( newPosition, forceSplitText );
+		return _breakAt( newPosition, forceSplitText );
 	}
 }
 
 // Unwraps children from provided `attribute`. Only children contained in `parent` element between
 // `startOffset` and `endOffset` will be unwrapped.
 //
-// @private
 // @param {engine.view.Element} parent
 // @param {Number} startOffset
 // @param {Number} endOffset
@@ -653,7 +623,7 @@ function unwrapChildren( parent, startOffset, endOffset, attribute ) {
 			continue;
 		}
 
-		const newPosition = mergeAttributes( position );
+		const newPosition = mergeAt( position );
 
 		// If nodes were merged - other merge offsets will change.
 		if ( !newPosition.isEqual( position ) ) {
@@ -667,8 +637,7 @@ function unwrapChildren( parent, startOffset, endOffset, attribute ) {
 
 // Wraps children with provided `attribute`. Only children contained in `parent` element between
 // `startOffset` and `endOffset` will be wrapped.
-
-// @private
+//
 // @param {engine.view.Element} parent
 // @param {Number} startOffset
 // @param {Number} endOffset
@@ -712,7 +681,7 @@ function wrapChildren( parent, startOffset, endOffset, attribute ) {
 			continue;
 		}
 
-		const newPosition = mergeAttributes( position );
+		const newPosition = mergeAt( position );
 
 		// If nodes were merged - other merge offsets will change.
 		if ( !newPosition.isEqual( position ) ) {
@@ -730,7 +699,6 @@ function wrapChildren( parent, startOffset, endOffset, attribute ) {
 //		<p>foo[]</p>  ->  <p>foo{}</p>
 //		<p>[]foo</p>  ->  <p>{}foo</p>
 //
-// @private
 // @param {engine.view.Position} position
 // @returns {engine.view.Position} Position located inside text node or same position if there is no text nodes
 // before or after position location.
@@ -756,7 +724,6 @@ function movePositionToTextNode( position ) {
 //		<p>{}foobar</p> -> <p>[]foobar</p>
 //		<p>foobar{}</p> -> <p>foobar[]</p>
 //
-// @private
 // @param {engine.view.Position} position Position that need to be placed inside text node.
 // @returns {engine.view.Position} New position after breaking text node.
 function breakTextNode( position ) {
@@ -783,7 +750,6 @@ function breakTextNode( position ) {
 
 // Merges two text nodes into first node. Removes second node and returns merge position.
 //
-// @private
 // @param {engine.view.Text} t1 First text node to merge. Data from second text node will be moved at the end of
 // this text node.
 // @param {engine.view.Text} t2 Second text node to merge. This node will be removed after merging.
@@ -802,7 +768,6 @@ function mergeTextNodes( t1, t2 ) {
 // When merging is possible - all attributes, styles and classes are moved from wrapper element to element being
 // wrapped.
 //
-// @private
 // @param {engine.view.AttributeElement} wrapper Wrapper AttributeElement.
 // @param {engine.view.AttributeElement} toWrap AttributeElement to wrap using wrapper element.
 // @returns {Boolean} Returns `true` if elements are merged.
@@ -863,7 +828,6 @@ function wrapAttributeElement( wrapper, toWrap ) {
 // Unwraps {@link engine.view.AttributeElement AttributeElement} from another by removing corresponding attributes,
 // classes and styles. All attributes, classes and styles from wrapper should be present inside element being unwrapped.
 //
-// @private
 // @param {engine.view.AttributeElement} wrapper Wrapper AttributeElement.
 // @param {engine.view.AttributeElement} toUnwrap AttributeElement to unwrap using wrapper element.
 // @returns {Boolean} Returns `true` if elements are unwrapped.
@@ -922,7 +886,6 @@ function unwrapAttributeElement( wrapper, toUnwrap ) {
 // (`start` and `end` positions are located inside same {@link engine.view.AttributeElement AttributeElement}),
 // starts on 0 offset and ends after last child node.
 //
-// @private
 // @param {engine.view.Range} Range
 // @returns {Boolean}
 function rangeSpansOnAllChildren( range ) {
@@ -939,7 +902,6 @@ function rangeSpansOnAllChildren( range ) {
 // {@link engine.view.AttributeElement AttributeElements} or
 // {@link engine.view.ContainerElement ContainerElements}.
 //
-// @private
 // @param Iterable.<engine.view.Text|engine.view.AttributeElement|engine.view.ContainerElement> nodes
 function validateNodesToInsert( nodes ) {
 	for ( let node of nodes ) {
@@ -959,11 +921,31 @@ function validateNodesToInsert( nodes ) {
 	}
 }
 
-// Checks if node is ContainerElement or DocumentFragment, because in most cases they should be thread the same way.
+// Checks if node is ContainerElement or DocumentFragment, because in most cases they should be treated the same way.
 //
-// @private
 // @param {engine.view.Node} node
 // @returns {Boolean} Returns `true` if node is instance of ContainerElement or DocumentFragment.
 function isContainerOrFragment( node ) {
 	return node instanceof ContainerElement || node instanceof DocumentFragment;
 }
+
+// Checks if {@link engine.view.Range#start range start} and {@link engine.view.Range#end range end} are placed
+// inside same {@link engine.view.ContainerElement container}.
+// Throws {@link utils.CKEditorError CKEditorError} `view-writer-invalid-range-container` when validation fails.
+//
+// @param {engine.view.Range} range
+function validateRangeContainer( range ) {
+	const startContainer = getParentContainer( range.start );
+	const endContainer = getParentContainer( range.end );
+
+	if ( !startContainer || !endContainer || startContainer !== endContainer ) {
+		/**
+		 * Range container is invalid. This can happen if {@link engine.view.Range#start range start} and
+		 * {@link engine.view.Range#end range end} positions are not placed inside same container or
+		 * parent container for this positions cannot be found.
+		 *
+		 * @error view-writer-invalid-range-container
+		 */
+		throw new CKEditorError( 'view-writer-invalid-range-container' );
+	}
+}

+ 2 - 2
packages/ckeditor5-engine/tests/conversion/model-selection-to-view-converters.js

@@ -15,7 +15,7 @@ import ModelPosition from '/ckeditor5/engine/model/position.js';
 import ViewDocument from '/ckeditor5/engine/view/document.js';
 import ViewContainerElement from '/ckeditor5/engine/view/containerelement.js';
 import ViewAttributeElement from '/ckeditor5/engine/view/attributeelement.js';
-import { mergeAttributes } from '/ckeditor5/engine/view/writer.js';
+import { mergeAt } from '/ckeditor5/engine/view/writer.js';
 
 import Mapper from '/ckeditor5/engine/conversion/mapper.js';
 import ModelConversionDispatcher from '/ckeditor5/engine/conversion/modelconversiondispatcher.js';
@@ -288,7 +288,7 @@ describe( 'clean-up', () => {
 			);
 
 			// Remove <b></b> manually.
-			mergeAttributes( viewSelection.getFirstPosition() );
+			mergeAt( viewSelection.getFirstPosition() );
 
 			const modelRange = ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 1 );
 			modelDoc.selection.setRanges( [ modelRange ] );

+ 3 - 3
packages/ckeditor5-engine/tests/view/writer/breakattributes.js → packages/ckeditor5-engine/tests/view/writer/breakAt.js

@@ -7,7 +7,7 @@
 
 'use strict';
 
-import { breakAttributes } from '/ckeditor5/engine/view/writer.js';
+import { breakAt } from '/ckeditor5/engine/view/writer.js';
 import DocumentFragment from '/ckeditor5/engine/view/documentfragment.js';
 import AttributeElement from '/ckeditor5/engine/view/attributeelement.js';
 import Text from '/ckeditor5/engine/view/text.js';
@@ -29,11 +29,11 @@ describe( 'writer', () => {
 			view = new DocumentFragment( view );
 		}
 
-		const newPosition = breakAttributes( selection.getFirstPosition() );
+		const newPosition = breakAt( selection.getFirstPosition() );
 		expect( stringify( view, newPosition, { showType: true, showPriority: true } ) ).to.equal( expected );
 	}
 
-	describe( 'breakAttributes', () => {
+	describe( 'breakAt', () => {
 		it( 'should not break text nodes if they are not in attribute elements - middle', () => {
 			test(
 				'<container:p>foo{}bar</container:p>',

+ 10 - 1
packages/ckeditor5-engine/tests/view/writer/breakrange.js

@@ -14,6 +14,7 @@ import AttributeElement from '/ckeditor5/engine/view/attributeelement.js';
 import Text from '/ckeditor5/engine/view/text.js';
 import Range from '/ckeditor5/engine/view/range.js';
 import { stringify, parse } from '/tests/engine/_utils/view.js';
+import CKEditorError from '/ckeditor5/utils/ckeditorerror.js';
 
 describe( 'writer', () => {
 	/**
@@ -40,7 +41,15 @@ describe( 'writer', () => {
 
 			expect( () => {
 				breakRange( Range.createFromParentsAndOffsets( p1, 0, p2, 0 ) );
-			} ).to.throw( 'view-writer-invalid-range-container' );
+			} ).to.throw( CKEditorError, 'view-writer-invalid-range-container' );
+		} );
+
+		it( 'should throw when range has no parent container', () => {
+			const el = new AttributeElement( 'b' );
+
+			expect( () => {
+				breakRange( Range.createFromParentsAndOffsets( el, 0, el, 0 ) );
+			} ).to.throw( CKEditorError, 'view-writer-invalid-range-container' );
 		} );
 
 		it( 'should not break text nodes if they are not in attribute elements', () => {

+ 0 - 39
packages/ckeditor5-engine/tests/view/writer/getparentcontainer.js

@@ -1,39 +0,0 @@
-/**
- * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-/* bender-tags: view, browser-only */
-
-'use strict';
-
-import { getParentContainer } from '/ckeditor5/engine/view/writer.js';
-import ContainerElement from '/ckeditor5/engine/view/containerelement.js';
-import AttributeElement from '/ckeditor5/engine/view/attributeelement.js';
-import Text from '/ckeditor5/engine/view/text.js';
-import Position from '/ckeditor5/engine/view/position.js';
-
-describe( 'writer', () => {
-	describe( 'getParentContainer', () => {
-		it( 'should return parent container of the node', () => {
-			const text = new Text( 'foobar' );
-			const b = new AttributeElement( 'b', null, [ text ] );
-			const parent = new ContainerElement( 'p', null, [ b ] );
-
-			b.priority = 1;
-			const container = getParentContainer( new Position( text, 0 ) );
-
-			expect( container ).to.equal( parent );
-		} );
-
-		it( 'should return undefined if no parent container', () => {
-			const text = new Text( 'foobar' );
-			const b = new AttributeElement( 'b', null, [ text ] );
-
-			b.priority = 1;
-			const container = getParentContainer( new Position( text, 0 ) );
-
-			expect( container ).to.be.undefined;
-		} );
-	} );
-} );

+ 10 - 0
packages/ckeditor5-engine/tests/view/writer/insert.js

@@ -165,5 +165,15 @@ describe( 'writer', () => {
 				insert( position, root );
 			} ).to.throw( CKEditorError, 'view-writer-insert-invalid-node' );
 		} );
+
+		it( 'should throw when position is not placed inside container', () => {
+			const element = new Element( 'b' );
+			const position = new Position( element, 0 );
+			const attributeElement = new AttributeElement( 'i' );
+
+			expect( () => {
+				insert( position, attributeElement );
+			} ).to.throw( CKEditorError, 'view-writer-invalid-position-container' );
+		} );
 	} );
 } );

+ 4 - 4
packages/ckeditor5-engine/tests/view/writer/mergeattributes.js → packages/ckeditor5-engine/tests/view/writer/mergeAt.js

@@ -7,7 +7,7 @@
 
 'use strict';
 
-import { mergeAttributes } from '/ckeditor5/engine/view/writer.js';
+import { mergeAt } from '/ckeditor5/engine/view/writer.js';
 import ContainerElement from '/ckeditor5/engine/view/containerelement.js';
 import Text from '/ckeditor5/engine/view/text.js';
 import Position from '/ckeditor5/engine/view/position.js';
@@ -23,11 +23,11 @@ describe( 'writer', () => {
 	 */
 	function test( input, expected ) {
 		const { view, selection } = parse( input );
-		const newPosition = mergeAttributes( selection.getFirstPosition() );
+		const newPosition = mergeAt( selection.getFirstPosition() );
 		expect( stringify( view, newPosition, { showType: true, showPriority: true } ) ).to.equal( expected );
 	}
 
-	describe( 'mergeAttributes', () => {
+	describe( 'mergeAt', () => {
 		it( 'should not merge if inside text node', () => {
 			test( '<container:p>fo{}bar</container:p>', '<container:p>fo{}bar</container:p>' );
 		} );
@@ -67,7 +67,7 @@ describe( 'writer', () => {
 			const p = new ContainerElement( 'p', null, [ t1, t2 ] );
 			const position = new Position( p, 1 );
 
-			const newPosition = mergeAttributes( position );
+			const newPosition = mergeAt( position );
 			expect( stringify( p, newPosition ) ).to.equal( '<p>foo{}bar</p>' );
 		} );
 

+ 10 - 1
packages/ckeditor5-engine/tests/view/writer/remove.js

@@ -14,6 +14,7 @@ import DocumentFragment from '/ckeditor5/engine/view/documentfragment.js';
 import { stringify, parse } from '/tests/engine/_utils/view.js';
 import AttributeElement from '/ckeditor5/engine/view/attributeelement.js';
 import Text from '/ckeditor5/engine/view/text.js';
+import CKEditorError from '/ckeditor5/utils/ckeditorerror.js';
 
 describe( 'writer', () => {
 	/**
@@ -44,7 +45,15 @@ describe( 'writer', () => {
 
 			expect( () => {
 				remove( Range.createFromParentsAndOffsets( p1, 0, p2, 0 ) );
-			} ).to.throw( 'view-writer-invalid-range-container' );
+			} ).to.throw( CKEditorError, 'view-writer-invalid-range-container' );
+		} );
+
+		it( 'should throw when range has no parent container', () => {
+			const el = new AttributeElement( 'b' );
+
+			expect( () => {
+				remove( Range.createFromParentsAndOffsets( el, 0, el, 0 ) );
+			} ).to.throw( CKEditorError, 'view-writer-invalid-range-container' );
 		} );
 
 		it( 'should return empty DocumentFragment when range is collapsed', () => {

+ 10 - 1
packages/ckeditor5-engine/tests/view/writer/unwrap.js

@@ -77,7 +77,16 @@ describe( 'writer', () => {
 			const b = new AttributeElement( 'b' );
 
 			expect( () => {
-				unwrap( range, b, 1 );
+				unwrap( range, b );
+			} ).to.throw( CKEditorError, 'view-writer-invalid-range-container' );
+		} );
+
+		it( 'should throw when range has no parent container', () => {
+			const el = new AttributeElement( 'b' );
+			const b = new AttributeElement( 'b' );
+
+			expect( () => {
+				unwrap( Range.createFromParentsAndOffsets( el, 0, el, 0 ), b );
 			} ).to.throw( CKEditorError, 'view-writer-invalid-range-container' );
 		} );
 

+ 10 - 1
packages/ckeditor5-engine/tests/view/writer/wrap.js

@@ -85,7 +85,16 @@ describe( 'writer', () => {
 			const b = new AttributeElement( 'b' );
 
 			expect( () => {
-				wrap( range, b, 1 );
+				wrap( range, b );
+			} ).to.throw( CKEditorError, 'view-writer-invalid-range-container' );
+		} );
+
+		it( 'should throw when range has no parent container', () => {
+			const el = new AttributeElement( 'b' );
+			const b = new AttributeElement( 'b' );
+
+			expect( () => {
+				wrap( Range.createFromParentsAndOffsets( el, 0, el, 0 ), b );
 			} ).to.throw( CKEditorError, 'view-writer-invalid-range-container' );
 		} );