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

Merge branch 'master' into t/ckeditor5/1660

Piotr Jasiun 6 лет назад
Родитель
Сommit
738b069851

+ 21 - 1
packages/ckeditor5-engine/src/model/differ.js

@@ -821,6 +821,26 @@ export default class Differ {
 					}
 				}
 
+				if ( old.type == 'remove' ) {
+					// This is a case when attribute change "contains" remove change.
+					// The attribute change needs to be split into two because changes cannot intersect.
+					if ( inc.offset < old.offset && incEnd > old.offset ) {
+						const attributePart = {
+							type: 'attribute',
+							offset: old.offset,
+							howMany: incEnd - old.offset,
+							count: this._changeCount++
+						};
+
+						this._handleChange( attributePart, changes );
+
+						changes.push( attributePart );
+
+						inc.nodesToHandle = old.offset - inc.offset;
+						inc.howMany = inc.nodesToHandle;
+					}
+				}
+
 				if ( old.type == 'attribute' ) {
 					// There are only two conflicting scenarios possible here:
 					if ( inc.offset >= old.offset && incEnd <= oldEnd ) {
@@ -1087,7 +1107,7 @@ function _generateActionsFromChanges( oldChildrenLength, changes ) {
 		} else {
 			actions.push( ...'a'.repeat( change.howMany ).split( '' ) );
 
-			// The last handled offset isa at the position after the changed range.
+			// The last handled offset is at the position after the changed range.
 			offset = change.offset + change.howMany;
 			// We changed `howMany` old nodes, update `oldChildrenHandled`.
 			oldChildrenHandled += change.howMany;

+ 70 - 0
packages/ckeditor5-engine/src/model/schema.js

@@ -39,6 +39,14 @@ export default class Schema {
 	constructor() {
 		this._sourceDefinitions = {};
 
+		/**
+		 * A dictionary containing attribute properties.
+		 *
+		 * @private
+		 * @member {Object.<String,String>}
+		 */
+		this._attributeProperties = {};
+
 		this.decorate( 'checkChild' );
 		this.decorate( 'checkAttribute' );
 
@@ -475,6 +483,58 @@ export default class Schema {
 		}, { priority: 'high' } );
 	}
 
+	/**
+	 * This method allows assigning additional metadata to the model attributes. For example,
+	 * {@link module:engine/model/schema~AttributeProperties `AttributeProperties#isFormatting` property} is
+	 * used to mark formatting attributes (like `bold` or `italic`).
+	 *
+	 *		// Mark bold as a formatting attribute.
+	 *		schema.setAttributeProperties( 'bold', {
+	 *			isFormatting: true
+	 *		} );
+	 *
+	 *		// Override code not to be considered a formatting markup.
+	 *		schema.setAttributeProperties( 'code', {
+	 *			isFormatting: false
+	 *		} );
+	 *
+	 * Properties are not limited to members defined in the
+	 * {@link module:engine/model/schema~AttributeProperties `AttributeProperties` type} and you can also use custom properties:
+	 *
+	 *		schema.setAttributeProperties( 'blockQuote', {
+	 *			customProperty: 'value'
+	 *		} );
+	 *
+	 * Subsequent calls with the same attribute will extend its custom properties:
+	 *
+	 *		schema.setAttributeProperties( 'blockQuote', {
+	 *			one: 1
+	 *		} );
+	 *
+	 *		schema.setAttributeProperties( 'blockQuote', {
+	 *			two: 2
+	 *		} );
+	 *
+	 *		console.log( schema.getAttributeProperties( 'blockQuote' ) );
+	 *		// Logs: { one: 1, two: 2 }
+	 *
+	 * @param {String} attributeName A name of the attribute to receive the properties.
+	 * @param {module:engine/model/schema~AttributeProperties} properties A dictionary of properties.
+	 */
+	setAttributeProperties( attributeName, properties ) {
+		this._attributeProperties[ attributeName ] = Object.assign( this.getAttributeProperties( attributeName ) || {}, properties );
+	}
+
+	/**
+	 * Returns properties associated with a given model attribute. See {@link #setAttributeProperties `setAttributeProperties()`}.
+	 *
+	 * @param {String} attributeName A name of the attribute.
+	 * @returns {module:engine/model/schema~AttributeProperties}
+	 */
+	getAttributeProperties( attributeName ) {
+		return this._attributeProperties[ attributeName ];
+	}
+
 	/**
 	 * Returns the lowest {@link module:engine/model/schema~Schema#isLimit limit element} containing the entire
 	 * selection/range/position or the root otherwise.
@@ -1285,6 +1345,16 @@ export class SchemaContext {
  * @typedef {Object} module:engine/model/schema~SchemaContextItem
  */
 
+/**
+ * A structure containing additional metadata describing the attribute.
+ *
+ * See {@link module:engine/model/schema~Schema#setAttributeProperties `Schema#setAttributeProperties()`} for usage examples.
+ *
+ * @typedef {Object} module:engine/model/schema~AttributeProperties
+ * @property {Boolean} [isFormatting] Indicates that the attribute should be considered as a visual formatting, like `bold`, `italic` or
+ * `fontSize` rather than semantic attribute (such as `src`, `listType`, etc.). For example, it is used by the "Remove format" feature.
+ */
+
 function compileBaseItemRule( sourceItemRules, itemName ) {
 	const itemRule = {
 		name: itemName,

+ 7 - 1
packages/ckeditor5-engine/src/model/utils/deletecontent.js

@@ -60,6 +60,13 @@ export default function deleteContent( model, selection, options = {} ) {
 		return;
 	}
 
+	const selRange = selection.getFirstRange();
+
+	// If the selection is already removed, don't do anything.
+	if ( selRange.root.rootName == '$graveyard' ) {
+		return;
+	}
+
 	const schema = model.schema;
 
 	model.change( writer => {
@@ -71,7 +78,6 @@ export default function deleteContent( model, selection, options = {} ) {
 			return;
 		}
 
-		const selRange = selection.getFirstRange();
 		const startPos = selRange.start;
 		const endPos = LivePosition.fromPosition( selRange.end, 'toNext' );
 

+ 4 - 14
packages/ckeditor5-engine/src/model/writer.js

@@ -814,22 +814,12 @@ export default class Writer {
 			throw new CKEditorError( 'writer-wrap-element-attached: Element to wrap with is already attached to tree model.' );
 		}
 
-		const version = range.root.document ? range.root.document.version : null;
-
-		// Has to be `range.start` not `range.end` for better transformations.
-		const insert = new InsertOperation( range.start, element, version );
-		this.batch.addOperation( insert );
-		this.model.applyOperation( insert );
+		this.insert( element, range.start );
 
-		const move = new MoveOperation(
-			range.start.getShiftedBy( 1 ),
-			range.end.offset - range.start.offset,
-			Position._createAt( element, 0 ),
-			version === null ? null : version + 1
-		);
+		// Shift the range-to-wrap because we just inserted an element before that range.
+		const shiftedRange = new Range( range.start.getShiftedBy( 1 ), range.end.getShiftedBy( 1 ) );
 
-		this.batch.addOperation( move );
-		this.model.applyOperation( move );
+		this.move( shiftedRange, Position._createAt( element, 0 ) );
 	}
 
 	/**

+ 41 - 1
packages/ckeditor5-engine/tests/model/differ.js

@@ -998,7 +998,7 @@ describe( 'Differ', () => {
 			} );
 		} );
 
-		it( 'remove and add attribute on text', () => {
+		it( 'remove attribute and add attribute on text', () => {
 			const p = root.getChild( 1 );
 
 			p.getChild( 0 )._setAttribute( 'bold', true );
@@ -1283,6 +1283,46 @@ describe( 'Differ', () => {
 				] );
 			} );
 		} );
+
+		it( 'add attribute after some text was removed', () => {
+			const p = root.getChild( 0 );
+
+			const range = new Range( Position._createAt( p, 0 ), Position._createAt( p, 2 ) );
+			const position = Position._createAt( p, 1 );
+
+			model.change( () => {
+				remove( position, 1 );
+				attribute( range, 'a', null, true );
+
+				const type = 'attribute';
+				const attributeOldValue = null;
+				const attributeNewValue = true;
+
+				// Attribute change glueing does not work 100% correct.
+				expectChanges( [
+					{
+						type,
+						range: new Range( Position._createAt( p, 0 ), Position._createAt( p, 1 ) ),
+						attributeKey: 'a',
+						attributeOldValue,
+						attributeNewValue
+					},
+					{
+						type: 'remove',
+						position,
+						length: 1,
+						name: '$text'
+					},
+					{
+						type,
+						range: new Range( Position._createAt( p, 1 ), Position._createAt( p, 2 ) ),
+						attributeKey: 'a',
+						attributeOldValue,
+						attributeNewValue
+					}
+				] );
+			} );
+		} );
 	} );
 
 	describe( 'split', () => {

+ 40 - 0
packages/ckeditor5-engine/tests/model/schema.js

@@ -96,6 +96,46 @@ describe( 'Schema', () => {
 		} );
 	} );
 
+	describe( 'setAttributeProperties()', () => {
+		beforeEach( () => {
+			schema.register( '$root' );
+			schema.register( 'paragraph', {
+				allowIn: '$root'
+			} );
+			schema.register( '$text', {
+				allowIn: 'paragraph'
+			} );
+			schema.extend( '$text', { allowAttributes: 'testAttribute' } );
+		} );
+
+		it( 'allows registering new properties', () => {
+			schema.setAttributeProperties( 'testAttribute', {
+				foo: 'bar',
+				baz: 'bom'
+			} );
+
+			expect( schema.getAttributeProperties( 'testAttribute' ) ).to.deep.equal( {
+				foo: 'bar',
+				baz: 'bom'
+			} );
+		} );
+
+		it( 'support adding properties in subsequent calls', () => {
+			schema.setAttributeProperties( 'testAttribute', {
+				first: 'foo'
+			} );
+
+			schema.setAttributeProperties( 'testAttribute', {
+				second: 'bar'
+			} );
+
+			expect( schema.getAttributeProperties( 'testAttribute' ) ).to.deep.equal( {
+				first: 'foo',
+				second: 'bar'
+			} );
+		} );
+	} );
+
 	describe( 'getDefinitions()', () => {
 		it( 'returns compiled definitions', () => {
 			schema.register( '$root' );

+ 23 - 0
packages/ckeditor5-engine/tests/model/utils/deletecontent.js

@@ -29,6 +29,29 @@ describe( 'DataController utils', () => {
 			} );
 		} );
 
+		it( 'should not do anything if the selection is already in graveyard', () => {
+			model = new Model();
+			doc = model.document;
+
+			const gy = model.document.graveyard;
+
+			gy._appendChild( new Element( 'paragraph' ) );
+
+			const baseVersion = model.document.baseVersion;
+
+			model.change( writer => {
+				sinon.spy( writer, 'remove' );
+
+				const selection = writer.createSelection( writer.createRangeIn( gy ) );
+
+				deleteContent( model, selection );
+
+				expect( writer.remove.called ).to.be.false;
+			} );
+
+			expect( model.document.baseVersion ).to.equal( baseVersion );
+		} );
+
 		describe( 'in simple scenarios', () => {
 			beforeEach( () => {
 				model = new Model();