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

Merge pull request #1011 from ckeditor/t/1001

Internal: Attributes which selection stores in elements should be removed once they are not empty any more. Closes #1001.
Szymon Cofalik 8 лет назад
Родитель
Сommit
0e29dc849c

+ 9 - 7
packages/ckeditor5-engine/src/model/document.js

@@ -18,7 +18,7 @@ import Position from './position';
 import RootElement from './rootelement';
 import Batch from './batch';
 import History from './history';
-import LiveSelection from './liveselection';
+import DocumentSelection from './documentselection';
 import Schema from './schema';
 import TreeWalker from './treewalker';
 import MarkerCollection from './markercollection';
@@ -90,9 +90,9 @@ export default class Document {
 		 * Selection done on this document.
 		 *
 		 * @readonly
-		 * @member {module:engine/model/liveselection~LiveSelection}
+		 * @member {module:engine/model/documentselection~DocumentSelection}
 		 */
-		this.selection = new LiveSelection( this );
+		this.selection = new DocumentSelection( this );
 
 		/**
 		 * Array of pending changes. See: {@link #enqueueChanges}.
@@ -368,7 +368,7 @@ export default class Document {
 		const json = clone( this );
 
 		// Due to circular references we need to remove parent reference.
-		json.selection = '[engine.model.LiveSelection]';
+		json.selection = '[engine.model.DocumentSelection]';
 
 		return json;
 	}
@@ -429,6 +429,8 @@ export default class Document {
 	 * * 'remove' when nodes are removed,
 	 * * 'reinsert' when remove is undone,
 	 * * 'move' when nodes are moved,
+	 * * 'rename' when element is renamed,
+	 * * 'marker' when a marker changes (added, removed or its range is changed),
 	 * * 'addAttribute' when attributes are added,
 	 * * 'removeAttribute' when attributes are removed,
 	 * * 'changeAttribute' when attributes change,
@@ -439,9 +441,9 @@ export default class Document {
 	 * @event change
 	 * @param {String} type Change type, possible option: 'insert', 'remove', 'reinsert', 'move', 'attribute'.
 	 * @param {Object} data Additional information about the change.
-	 * @param {module:engine/model/range~Range} data.range Range in model containing changed nodes. Note that the range state is
+	 * @param {module:engine/model/range~Range} [data.range] Range in model containing changed nodes. Note that the range state is
 	 * after changes has been done, i.e. for 'remove' the range will be in the {@link #graveyard graveyard root}.
-	 * This is `undefined` for "...root..." types.
+	 * The range is not defined for root, rename and marker types.
 	 * @param {module:engine/model/position~Position} [data.sourcePosition] Change source position.
 	 * Exists for 'remove', 'reinsert' and 'move'.
 	 * Note that this position state is before changes has been done, i.e. for 'reinsert' the source position will be in the
@@ -451,7 +453,7 @@ export default class Document {
 	 * 'changeRootAttribute' type.
 	 * @param {*} [data.newValue] Only for 'addAttribute', 'addRootAttribute', 'changeAttribute' or
 	 * 'changeRootAttribute' type.
-	 * @param {module:engine/model/rootelement~RootElement} [changeInfo.root] Root element which attributes got changed. This is defined
+	 * @param {module:engine/model/rootelement~RootElement} [data.root] Root element which attributes got changed. This is defined
 	 * only for root types.
 	 * @param {module:engine/model/batch~Batch} batch A {@link module:engine/model/batch~Batch batch}
 	 * of changes which this change is a part of.

+ 74 - 40
packages/ckeditor5-engine/src/model/liveselection.js → packages/ckeditor5-engine/src/model/documentselection.js

@@ -4,7 +4,7 @@
  */
 
 /**
- * @module engine/model/liveselection
+ * @module engine/model/documentselection
  */
 
 import Position from './position';
@@ -25,17 +25,20 @@ const attrOpTypes = new Set(
 );
 
 /**
- * `LiveSelection` is a type of {@link module:engine/model/selection~Selection selection} that listens to changes on a
- * {@link module:engine/model/document~Document document} and has it ranges updated accordingly. Internal implementation of this
- * mechanism bases on {@link module:engine/model/liverange~LiveRange live ranges}.
+ * `DocumentSelection` is a special selection which is used as the
+ * {@link module:engine/model/document~Document#selection document's selection}.
+ * There can be only one instance of `DocumentSelection` per document.
  *
- * Differences between {@link module:engine/model/selection~Selection} and `LiveSelection` are:
- * * there is always a range in `LiveSelection` - even if no ranges were added there is a
- * {@link module:engine/model/liveselection~LiveSelection#_getDefaultRange "default range"} present in the selection,
+ * `DocumentSelection` is automatically updated upon changes in the {@link module:engine/model/document~Document document}
+ * to always contain valid ranges. Its attributes are inherited from the text unless set explicitly.
+ *
+ * Differences between {@link module:engine/model/selection~Selection} and `DocumentSelection` are:
+ * * there is always a range in `DocumentSelection` - even if no ranges were added there is a "default range"
+ * present in the selection,
  * * ranges added to this selection updates automatically when the document changes,
- * * attributes of `LiveSelection` are updated automatically according to selection ranges.
+ * * attributes of `DocumentSelection` are updated automatically according to selection ranges.
  *
- * Since `LiveSelection` uses {@link module:engine/model/liverange~LiveRange live ranges}
+ * Since `DocumentSelection` uses {@link module:engine/model/liverange~LiveRange live ranges}
  * and is updated when {@link module:engine/model/document~Document document}
  * changes, it cannot be set on {@link module:engine/model/node~Node nodes}
  * that are inside {@link module:engine/model/documentfragment~DocumentFragment document fragment}.
@@ -44,7 +47,7 @@ const attrOpTypes = new Set(
  *
  * @extends module:engine/model/selection~Selection
  */
-export default class LiveSelection extends Selection {
+export default class DocumentSelection extends Selection {
 	/**
 	 * Creates an empty live selection for given {@link module:engine/model/document~Document}.
 	 *
@@ -57,7 +60,7 @@ export default class LiveSelection extends Selection {
 		 * Document which owns this selection.
 		 *
 		 * @protected
-		 * @member {module:engine/model/document~Document} module:engine/model/liveselection~LiveSelection#_document
+		 * @member {module:engine/model/document~Document} module:engine/model/documentselection~DocumentSelection#_document
 		 */
 		this._document = document;
 
@@ -65,20 +68,24 @@ export default class LiveSelection extends Selection {
 		 * Keeps mapping of attribute name to priority with which the attribute got modified (added/changed/removed)
 		 * last time. Possible values of priority are: `'low'` and `'normal'`.
 		 *
-		 * Priorities are used by internal `LiveSelection` mechanisms. All attributes set using `LiveSelection`
+		 * Priorities are used by internal `DocumentSelection` mechanisms. All attributes set using `DocumentSelection`
 		 * attributes API are set with `'normal'` priority.
 		 *
 		 * @private
-		 * @member {Map} module:engine/model/liveselection~LiveSelection#_attributePriority
+		 * @member {Map} module:engine/model/documentselection~DocumentSelection#_attributePriority
 		 */
 		this._attributePriority = new Map();
 
-		// Whenever attribute operation is performed on document, update selection attributes.
-		// This is not the most efficient way to update selection attributes, but should be okay for now.
-		this.listenTo( this._document, 'change', ( evt, type ) => {
+		this.listenTo( this._document, 'change', ( evt, type, changes, batch ) => {
+			// Whenever attribute operation is performed on document, update selection attributes.
+			// This is not the most efficient way to update selection attributes, but should be okay for now.
 			if ( attrOpTypes.has( type ) ) {
 				this._updateAttributes( false );
 			}
+
+			// Whenever element which had selection's attributes stored in it stops being empty,
+			// the attributes need to be removed.
+			clearAttributesStoredInElement( changes, batch );
 		} );
 	}
 
@@ -177,7 +184,7 @@ export default class LiveSelection extends Selection {
 	 */
 	setAttribute( key, value ) {
 		// Store attribute in parent element if the selection is collapsed in an empty node.
-		if ( this.isCollapsed && this.anchor.parent.childCount === 0 ) {
+		if ( this.isCollapsed && this.anchor.parent.isEmpty ) {
 			this._storeAttribute( key, value );
 		}
 
@@ -193,7 +200,7 @@ export default class LiveSelection extends Selection {
 	 */
 	removeAttribute( key ) {
 		// Remove stored attribute from parent element if the selection is collapsed in an empty node.
-		if ( this.isCollapsed && this.anchor.parent.childCount === 0 ) {
+		if ( this.isCollapsed && this.anchor.parent.isEmpty ) {
 			this._removeStoredAttribute( key );
 		}
 
@@ -210,7 +217,7 @@ export default class LiveSelection extends Selection {
 	setAttributesTo( attrs ) {
 		attrs = toMap( attrs );
 
-		if ( this.isCollapsed && this.anchor.parent.childCount === 0 ) {
+		if ( this.isCollapsed && this.anchor.parent.isEmpty ) {
 			this._setStoredAttributesTo( attrs );
 		}
 
@@ -238,17 +245,19 @@ export default class LiveSelection extends Selection {
 	}
 
 	/**
-	 * Creates and returns an instance of `LiveSelection` that is a clone of given selection, meaning that it has same
-	 * ranges and same direction as this selection.
-	 *
-	 * @params {module:engine/model/selection~Selection} otherSelection Selection to be cloned.
-	 * @returns {module:engine/model/liveselection~LiveSelection} `Selection` instance that is a clone of given selection.
+	 * This method is not available in `DocumentSelection`. There can be only one
+	 * `DocumentSelection` per document instance, so creating new `DocumentSelection`s this way
+	 * would be unsafe.
 	 */
-	static createFromSelection( otherSelection ) {
-		const selection = new this( otherSelection._document );
-		selection.setTo( otherSelection );
-
-		return selection;
+	static createFromSelection() {
+		/**
+		 * `DocumentSelection#createFromSelection()` is not available. There can be only one
+		 * `DocumentSelection` per document instance, so creating new `DocumentSelection`s this way
+		 * would be unsafe.
+		 *
+		 * @error documentselection-cannot-create
+		 */
+		throw new CKEditorError( 'documentselection-cannot-create: Cannot create new DocumentSelection instance.' );
 	}
 
 	/**
@@ -383,7 +392,7 @@ export default class LiveSelection extends Selection {
 	}
 
 	/**
-	 * Internal method for setting `LiveSelection` attribute. Supports attribute priorities (through `directChange`
+	 * Internal method for setting `DocumentSelection` attribute. Supports attribute priorities (through `directChange`
 	 * parameter).
 	 *
 	 * @private
@@ -417,7 +426,7 @@ export default class LiveSelection extends Selection {
 	}
 
 	/**
-	 * Internal method for removing `LiveSelection` attribute. Supports attribute priorities (through `directChange`
+	 * Internal method for removing `DocumentSelection` attribute. Supports attribute priorities (through `directChange`
 	 * parameter).
 	 *
 	 * @private
@@ -449,7 +458,7 @@ export default class LiveSelection extends Selection {
 	}
 
 	/**
-	 * Internal method for setting multiple `LiveSelection` attributes. Supports attribute priorities (through
+	 * Internal method for setting multiple `DocumentSelection` attributes. Supports attribute priorities (through
 	 * `directChange` parameter).
 	 *
 	 * @private
@@ -494,9 +503,9 @@ export default class LiveSelection extends Selection {
 	* _getStoredAttributes() {
 		const selectionParent = this.getFirstPosition().parent;
 
-		if ( this.isCollapsed && selectionParent.childCount === 0 ) {
+		if ( this.isCollapsed && selectionParent.isEmpty ) {
 			for ( const key of selectionParent.getAttributeKeys() ) {
-				if ( key.indexOf( storePrefix ) === 0 ) {
+				if ( key.startsWith( storePrefix ) ) {
 					const realKey = key.substr( storePrefix.length );
 
 					yield [ realKey, selectionParent.getAttribute( key ) ];
@@ -512,7 +521,7 @@ export default class LiveSelection extends Selection {
 	 * @param {String} key Key of attribute to remove.
 	 */
 	_removeStoredAttribute( key ) {
-		const storeKey = LiveSelection._getStoreAttributeKey( key );
+		const storeKey = DocumentSelection._getStoreAttributeKey( key );
 
 		this._document.batch().removeAttribute( this.anchor.parent, storeKey );
 	}
@@ -525,7 +534,7 @@ export default class LiveSelection extends Selection {
 	 * @param {*} value Attribute value.
 	 */
 	_storeAttribute( key, value ) {
-		const storeKey = LiveSelection._getStoreAttributeKey( key );
+		const storeKey = DocumentSelection._getStoreAttributeKey( key );
 
 		this._document.batch().setAttribute( this.anchor.parent, storeKey, value );
 	}
@@ -541,13 +550,13 @@ export default class LiveSelection extends Selection {
 		const batch = this._document.batch();
 
 		for ( const [ oldKey ] of this._getStoredAttributes() ) {
-			const storeKey = LiveSelection._getStoreAttributeKey( oldKey );
+			const storeKey = DocumentSelection._getStoreAttributeKey( oldKey );
 
 			batch.removeAttribute( selectionParent, storeKey );
 		}
 
 		for ( const [ key, value ] of attrs ) {
-			const storeKey = LiveSelection._getStoreAttributeKey( key );
+			const storeKey = DocumentSelection._getStoreAttributeKey( key );
 
 			batch.setAttribute( selectionParent, storeKey, value );
 		}
@@ -668,8 +677,9 @@ export default class LiveSelection extends Selection {
  * @event change:attribute
  */
 
-// Helper function for {@link module:engine/model/liveselection~LiveSelection#_updateAttributes}. It takes model item, checks whether
-// it is a text node (or text proxy) and if so, returns it's attributes. If not, returns `null`.
+// Helper function for {@link module:engine/model/documentselection~DocumentSelection#_updateAttributes}.
+//
+// It takes model item, checks whether it is a text node (or text proxy) and, if so, returns it's attributes. If not, returns `null`.
 //
 // @param {module:engine/model/item~Item|null}  node
 // @returns {Boolean}
@@ -680,3 +690,27 @@ function getAttrsIfCharacter( node ) {
 
 	return null;
 }
+
+// Removes selection attributes from element which is not empty anymore.
+function clearAttributesStoredInElement( changes, batch ) {
+	// Batch may not be passed to the document#change event in some tests.
+	// See https://github.com/ckeditor/ckeditor5-engine/issues/1001#issuecomment-314202352
+	// Ignore also transparent batches because they are... transparent.
+	if ( !batch || batch.type == 'transparent' ) {
+		return;
+	}
+
+	const changeParent = changes.range && changes.range.start.parent;
+
+	// `changes.range` is not set in case of rename, root and marker operations.
+	// None of them may lead to the element becoming non-empty.
+	if ( !changeParent || changeParent.isEmpty ) {
+		return;
+	}
+
+	const storedAttributes = Array.from( changeParent.getAttributeKeys() ).filter( key => key.startsWith( storePrefix ) );
+
+	for ( const key of storedAttributes ) {
+		batch.removeAttribute( changeParent, key );
+	}
+}

+ 6 - 0
packages/ckeditor5-engine/tests/model/_utils/utils.js

@@ -8,6 +8,7 @@ import TreeWalker from '../../../src/model/treewalker';
 import Text from '../../../src/model/text';
 import TextProxy from '../../../src/model/textproxy';
 import Delta from '../../../src/model/delta/delta';
+import Batch from '../../../src/model/batch';
 
 /**
  * Returns tree structure as a simplified string. Elements are uppercase and characters are lowercase.
@@ -57,7 +58,12 @@ export function jsonParseStringify( object ) {
  */
 export function wrapInDelta( operation ) {
 	const delta = new Delta();
+	// Batch() requires the document but only a few lines of code needs batch in `document#changes`
+	// so we may have an invalid batch instance for some tests.
+	const batch = new Batch();
+
 	delta.addOperation( operation );
+	batch.addDelta( delta );
 
 	return operation;
 }

+ 30 - 0
packages/ckeditor5-engine/tests/model/delta/transform/_utils/utils.js

@@ -10,6 +10,8 @@ import Text from '../../../../../src/model/text';
 import Position from '../../../../../src/model/position';
 import Range from '../../../../../src/model/range';
 
+import Batch from '../../../../../src/model/batch';
+
 import AttributeDelta from '../../../../../src/model/delta/attributedelta';
 import InsertDelta from '../../../../../src/model/delta/insertdelta';
 import WeakInsertDelta from '../../../../../src/model/delta/weakinsertdelta';
@@ -40,6 +42,8 @@ export function getInsertDelta( position, nodes, version ) {
 	const delta = new InsertDelta();
 	delta.addOperation( new InsertOperation( position, nodes, version ) );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -47,6 +51,8 @@ export function getWeakInsertDelta( position, nodes, version ) {
 	const delta = new WeakInsertDelta();
 	delta.addOperation( new InsertOperation( position, nodes, version ) );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -54,6 +60,8 @@ export function getMarkerDelta( name, oldRange, newRange, version ) {
 	const delta = new MarkerDelta();
 	delta.addOperation( new MarkerOperation( name, oldRange, newRange, version ) );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -77,6 +85,8 @@ export function getMergeDelta( position, howManyInPrev, howManyInNext, version )
 
 	delta.addOperation( new RemoveOperation( position, 1, gyPos, version + 1 ) );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -86,6 +96,8 @@ export function getMoveDelta( sourcePosition, howMany, targetPosition, baseVersi
 	const move = new MoveOperation( sourcePosition, howMany, targetPosition, baseVersion );
 	delta.addOperation( move );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -98,6 +110,8 @@ export function getRemoveDelta( sourcePosition, howMany, baseVersion ) {
 	const remove = new RemoveOperation( sourcePosition, howMany, gyPos, baseVersion );
 	delta.addOperation( remove );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -107,6 +121,8 @@ export function getRenameDelta( position, oldName, newName, baseVersion ) {
 	const rename = new RenameOperation( position, oldName, newName, baseVersion );
 	delta.addOperation( rename );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -127,6 +143,8 @@ export function getSplitDelta( position, nodeCopy, howManyMove, version ) {
 
 	delta.addOperation( move );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -142,6 +160,8 @@ export function getWrapDelta( range, element, version ) {
 	delta.addOperation( insert );
 	delta.addOperation( move );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -165,6 +185,8 @@ export function getUnwrapDelta( positionBefore, howManyChildren, version ) {
 	delta.addOperation( move );
 	delta.addOperation( remove );
 
+	wrapInBatch( delta );
+
 	return delta;
 }
 
@@ -221,3 +243,11 @@ export function getFilledDocument() {
 
 	return doc;
 }
+
+function wrapInBatch( delta ) {
+	// Batch() requires the document but only a few lines of code needs batch in `document#changes`
+	// so we may have an invalid batch instance for some tests.
+	const batch = new Batch();
+
+	batch.addDelta( delta );
+}

+ 1 - 1
packages/ckeditor5-engine/tests/model/document/document.js

@@ -558,6 +558,6 @@ describe( 'Document', () => {
 	} );
 
 	it( 'should be correctly converted to json', () => {
-		expect( jsonParseStringify( doc ).selection ).to.equal( '[engine.model.LiveSelection]' );
+		expect( jsonParseStringify( doc ).selection ).to.equal( '[engine.model.DocumentSelection]' );
 	} );
 } );

+ 153 - 30
packages/ckeditor5-engine/tests/model/liveselection.js → packages/ckeditor5-engine/tests/model/documentselection.js

@@ -9,7 +9,7 @@ import Text from '../../src/model/text';
 import Range from '../../src/model/range';
 import Position from '../../src/model/position';
 import LiveRange from '../../src/model/liverange';
-import LiveSelection from '../../src/model/liveselection';
+import DocumentSelection from '../../src/model/documentselection';
 import InsertOperation from '../../src/model/operation/insertoperation';
 import MoveOperation from '../../src/model/operation/moveoperation';
 import RemoveOperation from '../../src/model/operation/removeoperation';
@@ -25,9 +25,12 @@ import log from '@ckeditor/ckeditor5-utils/src/log';
 
 testUtils.createSinonSandbox();
 
-describe( 'LiveSelection', () => {
+describe( 'DocumentSelection', () => {
 	let doc, root, selection, liveRange, range;
 
+	const fooStoreAttrKey = DocumentSelection._getStoreAttributeKey( 'foo' );
+	const abcStoreAttrKey = DocumentSelection._getStoreAttributeKey( 'abc' );
+
 	beforeEach( () => {
 		doc = new Document();
 		root = doc.createRoot();
@@ -374,14 +377,16 @@ describe( 'LiveSelection', () => {
 	} );
 
 	describe( 'createFromSelection()', () => {
-		it( 'should return a LiveSelection instance', () => {
+		it( 'should throw', () => {
 			selection.addRange( range, true );
 
-			expect( LiveSelection.createFromSelection( selection ) ).to.be.instanceof( LiveSelection );
+			expect( () => {
+				DocumentSelection.createFromSelection( selection );
+			} ).to.throw( CKEditorError, /^documentselection-cannot-create:/ );
 		} );
 	} );
 
-	// LiveSelection uses LiveRanges so here are only simple test to see if integration is
+	// DocumentSelection uses LiveRanges so here are only simple test to see if integration is
 	// working well, without getting into complicated corner cases.
 	describe( 'after applying an operation should get updated and fire events', () => {
 		let spyRange;
@@ -532,6 +537,7 @@ describe( 'LiveSelection', () => {
 					expect( data.directChange ).to.be.false;
 				} );
 
+				const batch = doc.batch();
 				const splitDelta = new SplitDelta();
 
 				const insertOperation = new InsertOperation(
@@ -547,6 +553,8 @@ describe( 'LiveSelection', () => {
 					1
 				);
 
+				batch.addDelta( splitDelta );
+
 				splitDelta.addOperation( insertOperation );
 				splitDelta.addOperation( moveOperation );
 
@@ -679,7 +687,8 @@ describe( 'LiveSelection', () => {
 		let fullP, emptyP, rangeInFullP, rangeInEmptyP;
 
 		beforeEach( () => {
-			root.insertChildren( 0, [
+			root.removeChildren( 0, root.childCount );
+			root.appendChildren( [
 				new Element( 'p', [], new Text( 'foobar' ) ),
 				new Element( 'p', [], [] )
 			] );
@@ -689,6 +698,10 @@ describe( 'LiveSelection', () => {
 
 			rangeInFullP = new Range( new Position( root, [ 0, 4 ] ), new Position( root, [ 0, 4 ] ) );
 			rangeInEmptyP = new Range( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 0 ] ) );
+
+			// I've lost 30 mins debugging why my tests fail and that was due to the above code reusing
+			// a root which wasn't empty (so the ranges didn't actualy make much sense).
+			expect( root.childCount ).to.equal( 2 );
 		} );
 
 		describe( 'setAttribute()', () => {
@@ -698,7 +711,7 @@ describe( 'LiveSelection', () => {
 
 				expect( selection.getAttribute( 'foo' ) ).to.equal( 'bar' );
 
-				expect( emptyP.getAttribute( LiveSelection._getStoreAttributeKey( 'foo' ) ) ).to.equal( 'bar' );
+				expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
 			} );
 		} );
 
@@ -735,8 +748,8 @@ describe( 'LiveSelection', () => {
 				expect( selection.getAttribute( 'foo' ) ).to.equal( 'bar' );
 				expect( selection.getAttribute( 'abc' ) ).to.be.undefined;
 
-				expect( emptyP.getAttribute( LiveSelection._getStoreAttributeKey( 'foo' ) ) ).to.equal( 'bar' );
-				expect( emptyP.hasAttribute( LiveSelection._getStoreAttributeKey( 'abc' ) ) ).to.be.false;
+				expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
+				expect( emptyP.hasAttribute( abcStoreAttrKey ) ).to.be.false;
 			} );
 		} );
 
@@ -748,7 +761,7 @@ describe( 'LiveSelection', () => {
 
 				expect( selection.getAttribute( 'foo' ) ).to.be.undefined;
 
-				expect( fullP.hasAttribute( LiveSelection._getStoreAttributeKey( 'foo' ) ) ).to.be.false;
+				expect( fullP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
 			} );
 
 			it( 'should remove stored attribute if the selection is in empty node', () => {
@@ -758,7 +771,7 @@ describe( 'LiveSelection', () => {
 
 				expect( selection.getAttribute( 'foo' ) ).to.be.undefined;
 
-				expect( emptyP.hasAttribute( LiveSelection._getStoreAttributeKey( 'foo' ) ) ).to.be.false;
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
 			} );
 		} );
 
@@ -773,8 +786,8 @@ describe( 'LiveSelection', () => {
 				expect( selection.getAttribute( 'foo' ) ).to.be.undefined;
 				expect( selection.getAttribute( 'abc' ) ).to.be.undefined;
 
-				expect( emptyP.hasAttribute( LiveSelection._getStoreAttributeKey( 'foo' ) ) ).to.be.false;
-				expect( emptyP.hasAttribute( LiveSelection._getStoreAttributeKey( 'abc' ) ) ).to.be.false;
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
+				expect( emptyP.hasAttribute( abcStoreAttrKey ) ).to.be.false;
 			} );
 		} );
 
@@ -893,49 +906,159 @@ describe( 'LiveSelection', () => {
 			it( 'ignores attributes inside an object if selection contains that object', () => {
 				setData( doc, '<p>[<image><$text bold="true">Caption for the image.</$text></image>]</p>' );
 
-				const liveSelection = LiveSelection.createFromSelection( selection );
-
-				expect( liveSelection.hasAttribute( 'bold' ) ).to.equal( false );
+				expect( selection.hasAttribute( 'bold' ) ).to.equal( false );
 			} );
 
 			it( 'ignores attributes inside an object if selection contains that object (deeper structure)', () => {
 				setData( doc, '<p>[<image><caption><$text bold="true">Caption for the image.</$text></caption></image>]</p>' );
 
-				const liveSelection = LiveSelection.createFromSelection( selection );
-
-				expect( liveSelection.hasAttribute( 'bold' ) ).to.equal( false );
+				expect( selection.hasAttribute( 'bold' ) ).to.equal( false );
 			} );
 
 			it( 'ignores attributes inside an object if selection contains that object (block level)', () => {
 				setData( doc, '<p>foo</p>[<image><$text bold="true">Caption for the image.</$text></image>]<p>foo</p>' );
 
-				const liveSelection = LiveSelection.createFromSelection( selection );
-
-				expect( liveSelection.hasAttribute( 'bold' ) ).to.equal( false );
+				expect( selection.hasAttribute( 'bold' ) ).to.equal( false );
 			} );
 
 			it( 'reads attributes from text even if the selection contains an object', () => {
 				setData( doc, '<p>x<$text bold="true">[bar</$text><image></image>foo]</p>' );
 
-				const liveSelection = LiveSelection.createFromSelection( selection );
-
-				expect( liveSelection.getAttribute( 'bold' ) ).to.equal( true );
+				expect( selection.getAttribute( 'bold' ) ).to.equal( true );
 			} );
 
 			it( 'reads attributes when the entire selection inside an object', () => {
 				setData( doc, '<p><image><caption><$text bold="true">[bar]</$text></caption></image></p>' );
 
-				const liveSelection = LiveSelection.createFromSelection( selection );
-
-				expect( liveSelection.getAttribute( 'bold' ) ).to.equal( true );
+				expect( selection.getAttribute( 'bold' ) ).to.equal( true );
 			} );
 
 			it( 'stops reading attributes if selection starts with an object', () => {
 				setData( doc, '<p>[<image></image><$text bold="true">bar]</$text></p>' );
 
-				const liveSelection = LiveSelection.createFromSelection( selection );
+				expect( selection.hasAttribute( 'bold' ) ).to.equal( false );
+			} );
+		} );
+
+		describe( 'parent element\'s attributes', () => {
+			it( 'are set using a normal batch', () => {
+				const batchTypes = [];
+				doc.on( 'change', ( event, type, changes, batch ) => {
+					batchTypes.push( batch.type );
+				} );
+
+				selection.setRanges( [ rangeInEmptyP ] );
+				selection.setAttribute( 'foo', 'bar' );
+
+				expect( batchTypes ).to.deep.equal( [ 'default' ] );
+				expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
+			} );
+
+			it( 'are removed when any content is inserted (reuses the same batch)', () => {
+				// Dedupe batches by using a map (multiple change events will be fired).
+				const batchTypes = new Map();
+
+				selection.setRanges( [ rangeInEmptyP ] );
+				selection.setAttribute( 'foo', 'bar' );
+				selection.setAttribute( 'abc', 'bar' );
+
+				doc.on( 'change', ( event, type, changes, batch ) => {
+					batchTypes.set( batch, batch.type );
+				} );
+
+				doc.batch().insert( rangeInEmptyP.start, 'x' );
+
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
+				expect( emptyP.hasAttribute( abcStoreAttrKey ) ).to.be.false;
+
+				expect( Array.from( batchTypes.values() ) ).to.deep.equal( [ 'default' ] );
+			} );
+
+			it( 'are removed when any content is moved into', () => {
+				selection.setRanges( [ rangeInEmptyP ] );
+				selection.setAttribute( 'foo', 'bar' );
+
+				doc.batch().move( fullP.getChild( 0 ), rangeInEmptyP.start );
+
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
+			} );
+
+			it( 'are removed when containing element is merged with a non-empty element', () => {
+				const emptyP2 = new Element( 'p', null, 'x' );
+				root.appendChildren( emptyP2 );
+
+				emptyP.setAttribute( fooStoreAttrKey, 'bar' );
+				emptyP2.setAttribute( fooStoreAttrKey, 'bar' );
+
+				// <emptyP>{}<emptyP2>
+				doc.batch().merge( Position.createAfter( emptyP ) );
+
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
+				expect( emptyP.parent ).to.equal( root ); // Just to be sure we're checking the right element.
+			} );
+
+			it( 'are not removed or merged when containing element is merged with another empty element', () => {
+				const emptyP2 = new Element( 'p', null );
+				root.appendChildren( emptyP2 );
+
+				emptyP.setAttribute( fooStoreAttrKey, 'bar' );
+				emptyP2.setAttribute( abcStoreAttrKey, 'bar' );
+
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.true;
+				expect( emptyP.hasAttribute( abcStoreAttrKey ) ).to.be.false;
+
+				// <emptyP>{}<emptyP2>
+				doc.batch().merge( Position.createAfter( emptyP ) );
+
+				expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
+				expect( emptyP.parent ).to.equal( root ); // Just to be sure we're checking the right element.
+			} );
+
+			it( 'are removed even when there is no selection in it', () => {
+				emptyP.setAttribute( fooStoreAttrKey, 'bar' );
+
+				selection.setRanges( [ rangeInFullP ] );
+
+				doc.batch().insert( rangeInEmptyP.start, 'x' );
+
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
+			} );
+
+			it( 'are removed only once in case of multi-op deltas', () => {
+				const emptyP2 = new Element( 'p', null, 'x' );
+				root.appendChildren( emptyP2 );
+
+				emptyP.setAttribute( fooStoreAttrKey, 'bar' );
+				emptyP2.setAttribute( fooStoreAttrKey, 'bar' );
+
+				const batch = doc.batch();
+				const spy = sinon.spy( batch, 'removeAttribute' );
+
+				// <emptyP>{}<emptyP2>
+				batch.merge( Position.createAfter( emptyP ) );
+
+				expect( emptyP.hasAttribute( fooStoreAttrKey ) ).to.be.false;
+				expect( spy.calledOnce ).to.be.true;
+			} );
+
+			it( 'are not removed on transparent batches', () => {
+				selection.setRanges( [ rangeInEmptyP ] );
+				selection.setAttribute( 'foo', 'bar' );
+
+				doc.batch( 'transparent' ).insert( rangeInEmptyP.start, 'x' );
+
+				expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
+			} );
+
+			// Rename and some other deltas don't specify range in doc#change event.
+			// So let's see if there's no crash or something.
+			it( 'are not removed on rename', () => {
+				selection.setRanges( [ rangeInEmptyP ] );
+				selection.setAttribute( 'foo', 'bar' );
+
+				doc.batch().rename( emptyP, 'pnew' );
 
-				expect( liveSelection.hasAttribute( 'bold' ) ).to.equal( false );
+				expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' );
 			} );
 		} );
 	} );