Procházet zdrojové kódy

Merge branch 'master' into t/1399

Piotrek Koszuliński před 7 roky
rodič
revize
5ce67cdfde

+ 31 - 0
packages/ckeditor5-engine/CHANGELOG.md

@@ -1,6 +1,37 @@
 Changelog
 =========
 
+## [1.0.0-beta.2](https://github.com/ckeditor/ckeditor5-engine/compare/v1.0.0-beta.1...v1.0.0-beta.2) (2018-04-10)
+
+### Features
+
+* Introduced `writer#updateMarker()` method. Closes [#1299](https://github.com/ckeditor/ckeditor5-engine/issues/1299). ([bed647f](https://github.com/ckeditor/ckeditor5-engine/commit/bed647f))
+* Introduced `#isBefore` and `#isAfter` in `model.Node` and `view.Node`. Additionally, `model.Node#is()` and `view.Node#is()` and `view.Node#getPath()` were added. Closes [#1365](https://github.com/ckeditor/ckeditor5-engine/issues/1365). ([4c38683](https://github.com/ckeditor/ckeditor5-engine/commit/4c38683))
+
+### Bug fixes
+
+* `model.Differ` should not throw when multiple, intersecting remove changes were buffered. Closes [#1392](https://github.com/ckeditor/ckeditor5-engine/issues/1392). ([3a348fd](https://github.com/ckeditor/ckeditor5-engine/commit/3a348fd))
+* `model.Range#getTransformedByDelta` should not crash for `MoveDelta` which moves no nodes. Closes [#1358](https://github.com/ckeditor/ckeditor5-engine/issues/1358). ([ff8ba9e](https://github.com/ckeditor/ckeditor5-engine/commit/ff8ba9e))
+* `conversion.downcast-converters.changeAttribute` should not consume if element creators returned null. Closes [#1369](https://github.com/ckeditor/ckeditor5-engine/issues/1369). ([6866256](https://github.com/ckeditor/ckeditor5-engine/commit/6866256))
+* `conversion.downcast-converters.downcastAttributeToElement` should let specify from what element the model attribute will be converted. Closes [#1370](https://github.com/ckeditor/ckeditor5-engine/issues/1370). ([f8dec1e](https://github.com/ckeditor/ckeditor5-engine/commit/f8dec1e))
+
+### Other changes
+
+* Increased the specificity of CSS rules. Introduced the `.ck` class for editor UI components (see: [ckeditor/ckeditor5#494](https://github.com/ckeditor/ckeditor5/issues/494)). ([6bf32c0](https://github.com/ckeditor/ckeditor5-engine/commit/6bf32c0))
+* Refactored how markers removal is converted from the model to the view. Closes [#1226](https://github.com/ckeditor/ckeditor5-engine/issues/1226). ([f6de5f5](https://github.com/ckeditor/ckeditor5-engine/commit/f6de5f5))
+* Removed the unnecesary `model.Writer#setTextData()` method. Closes [#1363](https://github.com/ckeditor/ckeditor5-engine/issues/1363). ([b484822](https://github.com/ckeditor/ckeditor5-engine/commit/b484822))
+* Renamed plural method names to singular and singular attribute names to plural. See [ckeditor/ckeditor5#742](https://github.com/ckeditor/ckeditor5/issues/742). ([9465c82](https://github.com/ckeditor/ckeditor5-engine/commit/9465c82))
+* View selection is now split onto Selection and DocumentSelection. Closes [#1304](https://github.com/ckeditor/ckeditor5-engine/issues/1304) . ([b466e3f](https://github.com/ckeditor/ckeditor5-engine/commit/b466e3f))
+
+### BREAKING CHANGES
+
+* The `writer#setMarker()` method is used only to create a new marker and it does not accept a `marker` instance as a parameter. To update existing marker use `writer#updateMarker()` method.
+* The `options.usingOperation` option in `writer#setMarker()` is now a required one.
+* The `range` parameter was removed. Use `options.range` instead.
+* Properties in `MatcherPattern`, view `ElementDefinition` and options for conversion utils have been renamed: `class` to `classes`, `style` to `styles`, `attribute` to `attributes`.
+* Introduced `view.DocumentSelection`. It has protected API and can be modified only by the view writer. Observers creating instance of selection (like `SelectionObserver`, `MutationObserver`) use the `view.Selection` class now.
+
+
 ## [1.0.0-beta.1](https://github.com/ckeditor/ckeditor5-engine/compare/v1.0.0-alpha.2...v1.0.0-beta.1) (2018-03-15)
 
 ### Major refactoring

+ 14 - 14
packages/ckeditor5-engine/package.json

@@ -1,27 +1,27 @@
 {
   "name": "@ckeditor/ckeditor5-engine",
-  "version": "1.0.0-beta.1",
+  "version": "1.0.0-beta.2",
   "description": "CKEditor 5 editing engine.",
   "keywords": [
     "ckeditor5",
     "ckeditor5-lib"
   ],
   "dependencies": {
-    "@ckeditor/ckeditor5-utils": "^1.0.0-beta.1"
+    "@ckeditor/ckeditor5-utils": "^1.0.0-beta.2"
   },
   "devDependencies": {
-    "@ckeditor/ckeditor5-basic-styles": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-core": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-editor-classic": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-enter": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-essentials": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-heading": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-list": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-paragraph": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-typing": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-undo": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-widget": "^1.0.0-beta.1",
-    "@ckeditor/ckeditor5-link": "^1.0.0-beta.1",
+    "@ckeditor/ckeditor5-basic-styles": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-core": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-editor-classic": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-enter": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-essentials": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-heading": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-list": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-paragraph": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-typing": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-undo": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-widget": "^1.0.0-beta.2",
+    "@ckeditor/ckeditor5-link": "^1.0.0-beta.2",
     "eslint": "^4.15.0",
     "eslint-config-ckeditor5": "^1.0.7",
     "husky": "^0.14.3",

+ 9 - 0
packages/ckeditor5-engine/src/model/differ.js

@@ -690,6 +690,7 @@ export default class Differ {
 			}
 
 			if ( inc.type == 'attribute' ) {
+				// In case of attribute change, `howMany` should be kept same as `nodesToHandle`. It's not an error.
 				if ( old.type == 'insert' ) {
 					if ( inc.offset < old.offset && incEnd > old.offset ) {
 						if ( incEnd > oldEnd ) {
@@ -712,6 +713,7 @@ export default class Differ {
 						}
 
 						inc.nodesToHandle = old.offset - inc.offset;
+						inc.howMany = inc.nodesToHandle;
 					} else if ( inc.offset >= old.offset && inc.offset < oldEnd ) {
 						if ( incEnd > oldEnd ) {
 							inc.nodesToHandle = incEnd - oldEnd;
@@ -723,8 +725,15 @@ export default class Differ {
 				}
 
 				if ( old.type == 'attribute' ) {
+					// There are only two conflicting scenarios possible here:
 					if ( inc.offset >= old.offset && incEnd <= oldEnd ) {
+						// `old` change includes `inc` change, or they are the same.
 						inc.nodesToHandle = 0;
+						inc.howMany = 0;
+						inc.offset = 0;
+					} else if ( inc.offset <= old.offset && incEnd >= oldEnd ) {
+						// `inc` change includes `old` change.
+						old.howMany = 0;
 					}
 				}
 			}

+ 30 - 4
packages/ckeditor5-engine/src/view/writer.js

@@ -941,12 +941,13 @@ export default class Writer {
 			if ( isText || isEmpty || isUI || ( isAttribute && shouldABeOutsideB( attribute, child ) ) ) {
 				// Clone attribute.
 				const newAttribute = attribute._clone();
-				this._addToClonedElementsGroup( newAttribute );
 
 				// Wrap current node with new attribute.
 				child._remove();
 				newAttribute._appendChild( child );
+
 				parent._insertChild( i, newAttribute );
+				this._addToClonedElementsGroup( newAttribute );
 
 				wrapPositions.push(	new Position( parent, i ) );
 			}
@@ -1006,10 +1007,10 @@ export default class Writer {
 
 				// Replace wrapper element with its children
 				child._remove();
-				this._removeFromClonedElementsGroup( child );
-
 				parent._insertChild( i, unwrapped );
 
+				this._removeFromClonedElementsGroup( child );
+
 				// Save start and end position of moved items.
 				unwrapPositions.push(
 					new Position( parent, i ),
@@ -1415,10 +1416,10 @@ export default class Writer {
 
 			// Break element.
 			const clonedNode = positionParent._clone();
-			this._addToClonedElementsGroup( clonedNode );
 
 			// Insert cloned node to position's parent node.
 			positionParent.parent._insertChild( offsetAfter, clonedNode );
+			this._addToClonedElementsGroup( clonedNode );
 
 			// Get nodes to move.
 			const count = positionParent.childCount - positionOffset;
@@ -1447,6 +1448,19 @@ export default class Writer {
 	 * @param {module:engine/view/attributeelement~AttributeElement} element Attribute element to save.
 	 */
 	_addToClonedElementsGroup( element ) {
+		// Add only if the element is in document tree.
+		if ( !element.root.is( 'rootElement' ) ) {
+			return;
+		}
+
+		// Traverse the element's children recursively to find other attribute elements that also might got inserted.
+		// The loop is at the beginning so we can make fast returns later in the code.
+		if ( element.is( 'element' ) ) {
+			for ( const child of element.getChildren() ) {
+				this._addToClonedElementsGroup( child );
+			}
+		}
+
 		const id = element.id;
 
 		if ( !id ) {
@@ -1477,6 +1491,14 @@ export default class Writer {
 	 * @param {module:engine/view/attributeelement~AttributeElement} element Attribute element to remove.
 	 */
 	_removeFromClonedElementsGroup( element ) {
+		// Traverse the element's children recursively to find other attribute elements that also got removed.
+		// The loop is at the beginning so we can make fast returns later in the code.
+		if ( element.is( 'element' ) ) {
+			for ( const child of element.getChildren() ) {
+				this._removeFromClonedElementsGroup( child );
+			}
+		}
+
 		const id = element.id;
 
 		if ( !id ) {
@@ -1485,6 +1507,10 @@ export default class Writer {
 
 		const group = this._cloneGroups.get( id );
 
+		if ( !group ) {
+			return;
+		}
+
 		group.delete( element );
 		// Not removing group from element on purpose!
 		// If other parts of code have reference to this element, they will be able to get references to other elements from the group.

+ 134 - 0
packages/ckeditor5-engine/tests/model/differ.js

@@ -782,6 +782,140 @@ describe( 'Differ', () => {
 			} );
 		} );
 
+		it( 'attribute changes intersecting #1', () => {
+			const parent = root.getChild( 1 );
+
+			// Be aware that you cannot make an intersecting changes with the same attribute key,
+			// cause the value would be incorrect for the common part of the ranges.
+			const ranges = [
+				[ 0, 2, null, true, 'foo' ],
+				[ 1, 3, null, true, 'bar' ]
+			];
+
+			model.change( () => {
+				for ( const item of ranges ) {
+					const range = Range.createFromParentsAndOffsets( parent, item[ 0 ], parent, item[ 1 ] );
+
+					attribute( range, item[ 4 ], item[ 2 ], item[ 3 ] );
+				}
+
+				expectChanges( [
+					{
+						type: 'attribute',
+						range: Range.createFromParentsAndOffsets( parent, 0, parent, 2 ),
+						attributeKey: 'foo',
+						attributeOldValue: null,
+						attributeNewValue: true
+					},
+					{
+						type: 'attribute',
+						range: Range.createFromParentsAndOffsets( parent, 1, parent, 3 ),
+						attributeKey: 'bar',
+						attributeOldValue: null,
+						attributeNewValue: true
+					}
+				] );
+			} );
+		} );
+
+		it( 'attribute changes intersecting #2', () => {
+			const parent = root.getChild( 1 );
+
+			// Be aware that you cannot make an intersecting changes with the same attribute key,
+			// cause the value would be incorrect for the common part of the ranges.
+			const ranges = [
+				[ 1, 3, null, true, 'foo' ],
+				[ 0, 2, null, true, 'bar' ]
+			];
+
+			model.change( () => {
+				for ( const item of ranges ) {
+					const range = Range.createFromParentsAndOffsets( parent, item[ 0 ], parent, item[ 1 ] );
+
+					attribute( range, item[ 4 ], item[ 2 ], item[ 3 ] );
+				}
+
+				expectChanges( [
+					{
+						type: 'attribute',
+						range: Range.createFromParentsAndOffsets( parent, 0, parent, 1 ),
+						attributeKey: 'bar',
+						attributeOldValue: null,
+						attributeNewValue: true
+					},
+					{
+						type: 'attribute',
+						range: Range.createFromParentsAndOffsets( parent, 1, parent, 2 ),
+						attributeKey: 'foo',
+						attributeOldValue: null,
+						attributeNewValue: true
+					},
+					{
+						type: 'attribute',
+						range: Range.createFromParentsAndOffsets( parent, 1, parent, 2 ),
+						attributeKey: 'bar',
+						attributeOldValue: null,
+						attributeNewValue: true
+					},
+					{
+						type: 'attribute',
+						range: Range.createFromParentsAndOffsets( parent, 2, parent, 3 ),
+						attributeKey: 'foo',
+						attributeOldValue: null,
+						attributeNewValue: true
+					}
+				] );
+			} );
+		} );
+
+		it( 'attribute changes included in an attribute change #1 - changes are reversed at the end', () => {
+			const parent = root.getChild( 1 );
+
+			const ranges = [
+				[ 0, 1, null, true ],
+				[ 1, 2, null, true ],
+				[ 0, 2, true, null ]
+			];
+
+			model.change( () => {
+				for ( const item of ranges ) {
+					const range = Range.createFromParentsAndOffsets( parent, item[ 0 ], parent, item[ 1 ] );
+
+					attribute( range, attributeKey, item[ 2 ], item[ 3 ] );
+				}
+
+				expectChanges( [] );
+			} );
+		} );
+
+		it( 'attribute changes included in an attribute change #2 - changes are re-applied at the end', () => {
+			const parent = root.getChild( 1 );
+
+			const ranges = [
+				[ 0, 1, null, true ],
+				[ 1, 2, null, true ],
+				[ 0, 2, true, null ],
+				[ 0, 1, null, true ],
+				[ 1, 2, null, true ]
+			];
+
+			model.change( () => {
+				for ( const item of ranges ) {
+					const range = Range.createFromParentsAndOffsets( parent, item[ 0 ], parent, item[ 1 ] );
+
+					attribute( range, attributeKey, item[ 2 ], item[ 3 ] );
+				}
+
+				expectChanges( [ {
+					type: 'attribute',
+					range: Range.createFromParentsAndOffsets( parent, 0, parent, 2 ),
+					attributeKey,
+					attributeOldValue: null,
+					attributeNewValue: true
+				} ] );
+			} );
+		} );
+
 		it( 'on multiple non-consecutive characters in multiple operations', () => {
 			const parent = root.getChild( 0 );
 

+ 76 - 8
packages/ckeditor5-engine/tests/view/writer/writer.js

@@ -13,7 +13,7 @@ import createViewRoot from '../_utils/createroot';
 describe( 'Writer', () => {
 	let writer, attributes, root, doc;
 
-	before( () => {
+	beforeEach( () => {
 		attributes = { foo: 'bar', baz: 'quz' };
 		doc = new Document();
 		root = createViewRoot( doc );
@@ -43,6 +43,9 @@ describe( 'Writer', () => {
 
 	describe( 'setSelectionFocus()', () => {
 		it( 'should use selection._setFocus method internally', () => {
+			const position = ViewPosition.createAt( root );
+			writer.setSelection( position );
+
 			const spy = sinon.spy( writer.document.selection, '_setFocus' );
 			writer.setSelectionFocus( root, 0 );
 
@@ -255,10 +258,9 @@ describe( 'Writer', () => {
 
 	describe( 'manages AttributeElement#_clonesGroup', () => {
 		it( 'should return all clones of a broken attribute element with id', () => {
-			const container = writer.createContainerElement( 'div' );
 			const text = writer.createText( 'abccccde' );
 
-			writer.insert( ViewPosition.createAt( container, 0 ), text );
+			writer.insert( ViewPosition.createAt( root, 0 ), text );
 
 			const span = writer.createAttributeElement( 'span', null, { id: 'foo' } );
 			span._priority = 20;
@@ -271,8 +273,8 @@ describe( 'Writer', () => {
 			// <div>a<i>b<span>c</span></i><span>cc</span>de</div>
 			writer.wrap(
 				ViewRange.createFromParentsAndOffsets(
-					container.getChild( 0 ), 1,
-					container.getChild( 1 ).getChild( 0 ), 1
+					root.getChild( 0 ), 1,
+					root.getChild( 1 ).getChild( 0 ), 1
 				),
 				i
 			);
@@ -280,14 +282,14 @@ describe( 'Writer', () => {
 			// <div>a<i>b<span>c</span></i><span>c</span><i><span>cc</span>d</i>e</div>
 			writer.wrap(
 				ViewRange.createFromParentsAndOffsets(
-					container.getChild( 2 ).getChild( 0 ), 1,
-					container.getChild( 3 ), 1
+					root.getChild( 2 ).getChild( 0 ), 1,
+					root.getChild( 3 ), 1
 				),
 				i
 			);
 
 			// Find all spans.
-			const allSpans = Array.from( ViewRange.createIn( container ).getItems() ).filter( element => element.is( 'span' ) );
+			const allSpans = Array.from( ViewRange.createIn( root ).getItems() ).filter( element => element.is( 'span' ) );
 
 			// For each of the spans created above...
 			for ( const oneOfAllSpans of allSpans ) {
@@ -302,6 +304,72 @@ describe( 'Writer', () => {
 				expect( brokenArray.length ).to.equal( allSpans.length );
 			}
 		} );
+
+		it( 'should not create groups for attribute elements that are not created in document root', () => {
+			const p = writer.createContainerElement( 'p' );
+			const foo = writer.createText( 'foo' );
+			writer.insert( ViewPosition.createAt( p, 0 ), foo );
+			// <p>foo</p>
+
+			const span = writer.createAttributeElement( 'span', null, { id: 'span' } );
+
+			// <p><span>foo</span></p>
+			writer.wrap( ViewRange.createFromParentsAndOffsets( foo, 0, foo, 3 ), span );
+
+			// Find the span.
+			const createdSpan = p.getChild( 0 );
+
+			expect( createdSpan.getElementsWithSameId().size ).to.equal( 0 );
+		} );
+
+		it( 'should add attribute elements to clone groups deeply', () => {
+			const p = writer.createContainerElement( 'p' );
+			const foo = writer.createText( 'foo' );
+			writer.insert( ViewPosition.createAt( p, 0 ), foo );
+			// <p>foo</p>
+
+			const span = writer.createAttributeElement( 'span', null, { id: 'span' } );
+
+			// <p><span>foo</span></p>
+			writer.wrap( ViewRange.createFromParentsAndOffsets( foo, 0, foo, 3 ), span );
+
+			// <div><p><span>foo</span></p>
+			writer.insert( ViewPosition.createAt( root, 0 ), p );
+
+			// Find the span.
+			const createdSpan = p.getChild( 0 );
+
+			expect( Array.from( createdSpan.getElementsWithSameId() ) ).to.deep.equal( [ createdSpan ] );
+		} );
+
+		it( 'should remove attribute elements from clone groups deeply', () => {
+			const p1 = writer.createContainerElement( 'p' );
+			const p2 = writer.createContainerElement( 'p' );
+			const foo = writer.createText( 'foo' );
+			const bar = writer.createText( 'bar' );
+
+			writer.insert( ViewPosition.createAt( root, 0 ), p1 );
+			writer.insert( ViewPosition.createAt( root, 1 ), p2 );
+			writer.insert( ViewPosition.createAt( p1, 0 ), foo );
+			writer.insert( ViewPosition.createAt( p2, 0 ), bar );
+			// <div><p>foo</p><p>bar</p></div>
+
+			const span = writer.createAttributeElement( 'span', null, { id: 'span' } );
+
+			// <div><p>fo<span>o</span></p><p>bar</p></div>
+			writer.wrap( ViewRange.createFromParentsAndOffsets( foo, 2, foo, 3 ), span );
+
+			// <div><p>fo<span>o</span></p><p><span>b</span>ar</p></div>
+			writer.wrap( ViewRange.createFromParentsAndOffsets( bar, 0, bar, 1 ), span );
+
+			// <div><p><span>b</span>ar</p></div>
+			writer.remove( ViewRange.createOn( p1 ) );
+
+			// Find the span.
+			const spanInTree = p2.getChild( 0 );
+
+			expect( Array.from( spanInTree.getElementsWithSameId() ) ).to.deep.equal( [ spanInTree ] );
+		} );
 	} );
 
 	function assertElementAttributes( element, attributes ) {