DocumentSelection attributes update fix WIP.

src/model/documentselection.js

@@ -19,10 +19,6 @@ import toMap from '@ckeditor/ckeditor5-utils/src/tomap';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import log from '@ckeditor/ckeditor5-utils/src/log';
 
-const attrOpTypes = new Set(
-	[ 'addAttribute', 'removeAttribute', 'changeAttribute', 'addRootAttribute', 'removeRootAttribute', 'changeRootAttribute' ]
-);
-
 const storePrefix = 'selection:';
 
 /**
@@ -531,29 +527,13 @@ class LiveSelection extends Selection {
 			}
 		} );
 
-		this.listenTo( this._model, 'applyOperation', ( evt, args ) => {
-			const operation = args[ 0 ];
-
-			if ( !operation.isDocumentOperation ) {
-				return;
-			}
+		this.listenTo( this._document, 'change', ( evt, batch ) => {
+			// Update selection's attributes.
+			this._updateAttributes( false );
 
-			// 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( operation.type ) ) {
-				this._updateAttributes( false );
-			}
-
-			const batch = operation.delta.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
-			if ( batch ) {
-				// Whenever element which had selection's attributes stored in it stops being empty,
-				// the attributes need to be removed.
-				clearAttributesStoredInElement( operation, this._model, batch );
-			}
-		}, { priority: 'low' } );
+			// Clear selection attributes from element if no longer empty,
+			clearAttributesStoredInElement( this._model, batch );
+		} );
 
 		this.listenTo( this._model, 'applyOperation', () => {
 			while ( this._fixGraveyardRangesData.length ) {
@@ -1019,24 +999,27 @@ function getAttrsIfCharacter( node ) {
 }
 
 // Removes selection attributes from element which is not empty anymore.
-function clearAttributesStoredInElement( operation, model, batch ) {
-	let changeParent = null;
+function clearAttributesStoredInElement( model, batch ) {
+	// Clear attributes stored in selection;
+	const differ = model.document.differ;
 
-	if ( operation.type == 'insert' ) {
-		changeParent = operation.position.parent;
-	} else if ( operation.type == 'move' || operation.type == 'reinsert' || operation.type == 'remove' ) {
-		changeParent = operation.getMovedRangeStart().parent;
-	}
+	for ( const entry of differ.getChanges() ) {
+		if ( entry.type != 'insert' || !entry.position.parent ) {
+			continue;
+		}
 
-	if ( !changeParent || changeParent.isEmpty ) {
-		return;
-	}
+		const changeParent = entry.position.parent;
+		const isNoLongerEmpty = entry.length === changeParent.maxOffset;
 
-	model.enqueueChange( batch, writer => {
-		const storedAttributes = Array.from( changeParent.getAttributeKeys() ).filter( key => key.startsWith( storePrefix ) );
+		if ( isNoLongerEmpty ) {
+			model.enqueueChange( batch, writer => {
+				const storedAttributes = Array.from( changeParent.getAttributeKeys() )
+					.filter( key => key.startsWith( storePrefix ) );
 
-		for ( const key of storedAttributes ) {
-			writer.removeAttribute( key, changeParent );
+				for ( const key of storedAttributes ) {
+					writer.removeAttribute( key, changeParent );
+				}
+			} );
 		}
-	} );
+	}
 }

tests/model/documentselection.js

@@ -1056,15 +1056,10 @@ describe( 'DocumentSelection', () => {
 					expect( data.attributeKeys ).to.deep.equal( [ 'foo' ] );
 				} );
 
-				model.applyOperation( wrapInDelta(
-					new AttributeOperation(
-						new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 5 ] ) ),
-						'foo',
-						null,
-						'bar',
-						doc.version
-					)
-				) );
+				model.change( writer => {
+					const range = new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 5 ] ) );
+					writer.setAttribute( 'foo', 'bar', range );
+				} );
 
 				expect( selection.getAttribute( 'foo' ) ).to.equal( 'bar' );
 				expect( spyAttribute.calledOnce ).to.be.true;