Merge pull request #14322 from ckeditor/ck/14216-change-htmlListAttri…

…butes

Feature (html-support): Changing list type removes styles from the old type. Closes #14216.

packages/ckeditor5-html-support/src/integrations/documentlist.ts

@@ -18,7 +18,7 @@ import type {
 	DocumentListIndentCommand
 } from '@ckeditor/ckeditor5-list';
 
-import { type GHSViewAttributes, setViewAttributes } from '../utils';
+import { getHtmlAttributeName, setViewAttributes } from '../utils';
 import DataFilter, { type DataFilterRegisterEvent } from '../datafilter';
 
 /**
@@ -53,49 +53,52 @@ export default class DocumentListElementSupport extends Plugin {
 		const conversion = editor.conversion;
 		const dataFilter = editor.plugins.get( DataFilter );
 		const documentListEditing: DocumentListEditing = editor.plugins.get( 'DocumentListEditing' );
+		const viewElements = [ 'ul', 'ol', 'li' ];
 
 		// Register downcast strategy.
 		// Note that this must be done before document list editing registers conversion in afterInit.
 		documentListEditing.registerDowncastStrategy( {
 			scope: 'item',
 			attributeName: 'htmlLiAttributes',
-
-			setAttributeOnDowncast( writer, attributeValue: GHSViewAttributes, viewElement ) {
-				setViewAttributes( writer, attributeValue, viewElement );
-			}
+			setAttributeOnDowncast: setViewAttributes
 		} );
 
 		documentListEditing.registerDowncastStrategy( {
 			scope: 'list',
-			attributeName: 'htmlListAttributes',
+			attributeName: 'htmlUlAttributes',
+			setAttributeOnDowncast: setViewAttributes
+		} );
 
-			setAttributeOnDowncast( writer, viewAttributes: GHSViewAttributes, viewElement ) {
-				setViewAttributes( writer, viewAttributes, viewElement );
-			}
+		documentListEditing.registerDowncastStrategy( {
+			scope: 'list',
+			attributeName: 'htmlOlAttributes',
+			setAttributeOnDowncast: setViewAttributes
 		} );
 
 		dataFilter.on<DataFilterRegisterEvent>( 'register', ( evt, definition ) => {
-			if ( ![ 'ul', 'ol', 'li' ].includes( definition.view! ) ) {
+			if ( !viewElements.includes( definition.view! ) ) {
 				return;
 			}
 
 			evt.stop();
 
 			// Do not register same converters twice.
-			if ( schema.checkAttribute( '$block', 'htmlListAttributes' ) ) {
+			if ( schema.checkAttribute( '$block', 'htmlLiAttributes' ) ) {
 				return;
 			}
 
-			schema.extend( '$block', { allowAttributes: [ 'htmlListAttributes', 'htmlLiAttributes' ] } );
-			schema.extend( '$blockObject', { allowAttributes: [ 'htmlListAttributes', 'htmlLiAttributes' ] } );
-			schema.extend( '$container', { allowAttributes: [ 'htmlListAttributes', 'htmlLiAttributes' ] } );
+			const allowAttributes = viewElements.map( element => getHtmlAttributeName( element ) );
+
+			schema.extend( '$block', { allowAttributes } );
+			schema.extend( '$blockObject', { allowAttributes } );
+			schema.extend( '$container', { allowAttributes } );
 
 			conversion.for( 'upcast' ).add( dispatcher => {
 				dispatcher.on<UpcastElementEvent>(
-					'element:ul', viewToModelListAttributeConverter( 'htmlListAttributes', dataFilter ), { priority: 'low' }
+					'element:ul', viewToModelListAttributeConverter( 'htmlUlAttributes', dataFilter ), { priority: 'low' }
 				);
 				dispatcher.on<UpcastElementEvent>(
-					'element:ol', viewToModelListAttributeConverter( 'htmlListAttributes', dataFilter ), { priority: 'low' }
+					'element:ol', viewToModelListAttributeConverter( 'htmlOlAttributes', dataFilter ), { priority: 'low' }
 				);
 				dispatcher.on<UpcastElementEvent>(
 					'element:li', viewToModelListAttributeConverter( 'htmlLiAttributes', dataFilter ), { priority: 'low' }
@@ -140,10 +143,11 @@ export default class DocumentListElementSupport extends Plugin {
 				}
 
 				if ( previousNodeInList.getAttribute( 'listType' ) == node.getAttribute( 'listType' ) ) {
-					const value = previousNodeInList.getAttribute( 'htmlListAttributes' );
+					const attribute = getAttributeFromListType( previousNodeInList.getAttribute( 'listType' ) );
+					const value = previousNodeInList.getAttribute( attribute );
 
-					if ( !isEqual( node.getAttribute( 'htmlListAttributes' ), value ) ) {
-						writer.setAttribute( 'htmlListAttributes', value, node );
+					if ( !isEqual( node.getAttribute( attribute ), value ) ) {
+						writer.setAttribute( attribute, value, node );
 						evt.return = true;
 					}
 				}
@@ -158,6 +162,23 @@ export default class DocumentListElementSupport extends Plugin {
 				}
 			}
 		} );
+
+		// Remove `ol` attributes from `ul` elements and vice versa.
+		documentListEditing.on<DocumentListEditingPostFixerEvent>( 'postFixer', ( evt, { listNodes, writer } ) => {
+			for ( const { node } of listNodes ) {
+				const listType = node.getAttribute( 'listType' );
+
+				if ( listType === 'bulleted' && node.getAttribute( 'htmlOlAttributes' ) ) {
+					writer.removeAttribute( 'htmlOlAttributes', node );
+					evt.return = true;
+				}
+
+				if ( listType === 'numbered' && node.getAttribute( 'htmlUlAttributes' ) ) {
+					writer.removeAttribute( 'htmlUlAttributes', node );
+					evt.return = true;
+				}
+			}
+		} );
 	}
 
 	/**
@@ -175,10 +196,12 @@ export default class DocumentListElementSupport extends Plugin {
 		this.listenTo( indentList, 'afterExecute', ( evt, changedBlocks ) => {
 			editor.model.change( writer => {
 				for ( const node of changedBlocks ) {
+					const attribute = getAttributeFromListType( node.getAttribute( 'listType' ) );
+
 					// Just reset the attribute.
 					// If there is a previous indented list that this node should be merged into,
 					// the postfixer will unify all the attributes of both sub-lists.
-					writer.setAttribute( 'htmlListAttributes', {}, node );
+					writer.setAttribute( attribute, {}, node );
 				}
 			} );
 		} );
@@ -191,8 +214,8 @@ export default class DocumentListElementSupport extends Plugin {
  *
  * @returns Returns a conversion callback.
  */
-function viewToModelListAttributeConverter( attributeName: string, dataFilter: DataFilter ) {
-	const callback: GetCallback<UpcastElementEvent> = ( evt, data, conversionApi ) => {
+function viewToModelListAttributeConverter( attributeName: string, dataFilter: DataFilter ): GetCallback<UpcastElementEvent> {
+	return ( evt, data, conversionApi ) => {
 		const viewElement = data.viewItem;
 
 		if ( !data.modelRange ) {
@@ -216,6 +239,13 @@ function viewToModelListAttributeConverter( attributeName: string, dataFilter: D
 			conversionApi.writer.setAttribute( attributeName, viewAttributes || {}, item );
 		}
 	};
+}
 
-	return callback;
+/**
+ * Returns HTML attribute name based on provided list type.
+ */
+function getAttributeFromListType( listType: 'bulleted' | 'numbered' ) {
+	return listType === 'bulleted' ?
+		'htmlUlAttributes' :
+		'htmlOlAttributes';
 }

packages/ckeditor5-html-support/src/schemadefinitions.ts

@@ -524,13 +524,13 @@ export default {
 			coupledAttribute: 'listItemId'
 		},
 		{
-			model: 'htmlListAttributes',
+			model: 'htmlOlAttributes',
 			view: 'ol',
 			appliesToBlock: true,
 			coupledAttribute: 'listItemId'
 		},
 		{
-			model: 'htmlListAttributes',
+			model: 'htmlUlAttributes',
 			view: 'ul',
 			appliesToBlock: true,
 			coupledAttribute: 'listItemId'

packages/ckeditor5-html-support/tests/generalhtmlsupport.js

@@ -58,8 +58,8 @@ describe( 'GeneralHtmlSupport', () => {
 		} );
 
 		it( 'should return model attribute name for list elements with multiple view representations', () => {
-			expect( generalHtmlSupport.getGhsAttributeNameForElement( 'ul' ) ).to.equal( 'htmlListAttributes' );
-			expect( generalHtmlSupport.getGhsAttributeNameForElement( 'ol' ) ).to.equal( 'htmlListAttributes' );
+			expect( generalHtmlSupport.getGhsAttributeNameForElement( 'ul' ) ).to.equal( 'htmlUlAttributes' );
+			expect( generalHtmlSupport.getGhsAttributeNameForElement( 'ol' ) ).to.equal( 'htmlOlAttributes' );
 			expect( generalHtmlSupport.getGhsAttributeNameForElement( 'li' ) ).to.equal( 'htmlLiAttributes' );
 		} );