Fix upcasting of background-color in tables. (#18808)

* Restore `shouldUpcast` method due to incorrect upcast of background styling in tables.

* Invert condition

* Add tests.

* Add symmetry test.

Author: Mati365

packages/ckeditor5-table/src/converters/tableproperties.ts

@@ -32,6 +32,7 @@ export function upcastStyleToAttribute(
 		viewElement: string | RegExp;
 		defaultValue: string;
 		reduceBoxSides?: boolean;
+		shouldUpcast?: ( viewElement: ViewElement ) => boolean;
 	}
 ): void {
 	const {
@@ -41,6 +42,7 @@ export function upcastStyleToAttribute(
 		attributeType,
 		viewElement,
 		defaultValue,
+		shouldUpcast = () => true,
 		reduceBoxSides = false
 	} = options;
 
@@ -55,10 +57,7 @@ export function upcastStyleToAttribute(
 			key: modelAttribute,
 			value: ( viewElement: ViewElement, conversionApi: UpcastConversionApi, data: UpcastConversionData<ViewElement> ) => {
 				// Ignore table elements inside figures and figures without the table class.
-				if (
-					viewElement.name == 'table' && viewElement.parent!.name == 'figure' ||
-					viewElement.name == 'figure' && !viewElement.hasClass( 'table' )
-				) {
+				if ( !shouldUpcast( viewElement ) ) {
 					return;
 				}
 

packages/ckeditor5-table/src/tableproperties/tablepropertiesediting.ts

@@ -363,6 +363,14 @@ function enableTableToFigureProperty(
 
 	schema.setAttributeProperties( modelAttribute, { isFormatting: true } );
 
-	upcastStyleToAttribute( conversion, { viewElement: /^(table|figure)$/, ...options } );
+	upcastStyleToAttribute( conversion, {
+		viewElement: /^(table|figure)$/,
+		shouldUpcast: ( viewElement: ViewElement ) => !(
+			viewElement.name == 'table' && viewElement.parent!.name == 'figure' ||
+			viewElement.name == 'figure' && !viewElement.hasClass( 'table' )
+		),
+		...options
+	} );
+
 	downcastAttributeToStyle( conversion, { modelElement: 'table', ...options } );
 }

packages/ckeditor5-table/tests/tableproperties/tablepropertiesediting.js

@@ -899,6 +899,24 @@ describe( 'table properties', () => {
 					expect( table.getAttribute( 'tableBackgroundColor' ) ).to.equal( '#f00' );
 				} );
 
+				it( 'should upcast background-color from table within <figure> with `.table` class', () => {
+					editor.setData(
+						'<figure class="table"><table style="background-color:#f00"><tr><td>foo</td></tr></table></figure>'
+					);
+					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
+
+					expect( table.getAttribute( 'tableBackgroundColor' ) ).to.equal( '#f00' );
+				} );
+
+				it( 'should upcast background-color from table without <figure> without `.table` class', () => {
+					editor.setData(
+						'<figure><table style="background-color:#f00"><tr><td>foo</td></tr></table></figure>'
+					);
+					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
+
+					expect( table.getAttribute( 'tableBackgroundColor' ) ).to.equal( '#f00' );
+				} );
+
 				it( 'should upcast from background shorthand', () => {
 					editor.setData( '<table style="background:#f00 center center"><tr><td>foo</td></tr></table>' );
 					const table = model.document.getRoot().getNodeByPath( [ 0 ] );
@@ -1024,6 +1042,27 @@ describe( 'table properties', () => {
 					assertTableStyle( editor, 'background-color:#ba7;' );
 				} );
 			} );
+
+			it( 'data symmetry - should upcast downcasted tableBackgroundColor', () => {
+				editor.setData( '<table style="background-color:#f00"><tr><td>foo</td></tr></table>' );
+				const table = model.document.getRoot().getNodeByPath( [ 0 ] );
+
+				expect( table.getAttribute( 'tableBackgroundColor' ) ).to.equal( '#f00' );
+
+				model.change( writer => writer.setAttribute( 'tableBackgroundColor', '#ba7', table ) );
+
+				expect( editor.getData() ).to.equalMarkup(
+					'<figure class="table">' +
+						'<table style="background-color:#ba7;">' +
+							'<tbody>' +
+								'<tr>' +
+									'<td>foo</td>' +
+								'</tr>' +
+							'</tbody>' +
+						'</table>' +
+					'</figure>'
+				);
+			} );
 		} );
 
 		describe( 'tableWidth', () => {