diff --git a/packages/ckeditor5-table/src/converters/upcasttable.ts b/packages/ckeditor5-table/src/converters/upcasttable.ts index 2c5fc63bc2b..3de4dd2f860 100644 --- a/packages/ckeditor5-table/src/converters/upcasttable.ts +++ b/packages/ckeditor5-table/src/converters/upcasttable.ts @@ -241,14 +241,26 @@ function scanTable( viewTable: ViewElement ) { ( el: ViewNode ): el is ViewElement & { name: 'tr' } => el.is( 'element', 'tr' ) ); + // Keep tracking of the previous row columns count to improve detection of heading rows. + let maxPrevColumns = null; + for ( const tr of trs ) { + const trColumns = Array + .from( tr.getChildren() ) + .filter( el => el.is( 'element', 'td' ) || el.is( 'element', 'th' ) ); + // This is a child of a first element. if ( ( firstTheadElement && tableChild === firstTheadElement ) || ( tableChild.name === 'tbody' && - Array.from( tr.getChildren() ).length && - Array.from( tr.getChildren() ).every( e => e.is( 'element', 'th' ) ) + trColumns.length > 0 && + // These conditions handles the case when the first column is a element and it's the only column in the row. + // This case is problematic because it's not clear if this row should be a heading row or not, as it may be result + // of the cell span from the previous row. + // Issue: https://github.com/ckeditor/ckeditor5/issues/17556 + ( maxPrevColumns === null || trColumns.length === maxPrevColumns ) && + trColumns.every( e => e.is( 'element', 'th' ) ) ) ) { headingRows++; @@ -263,6 +275,11 @@ function scanTable( viewTable: ViewElement ) { headingColumns = headingCols; } } + + // We use the maximum number of columns to avoid false positives when detecting + // multiple rows with single column within `rowspan`. Without it the last row of `rowspan=3` + // would be detected as a heading row because it has only one column (identical to the previous row). + maxPrevColumns = Math.max( maxPrevColumns || 0, trColumns.length ); } } diff --git a/packages/ckeditor5-table/tests/converters/upcasttable.js b/packages/ckeditor5-table/tests/converters/upcasttable.js index 0bf5af058ac..1744d6cd4cc 100644 --- a/packages/ckeditor5-table/tests/converters/upcasttable.js +++ b/packages/ckeditor5-table/tests/converters/upcasttable.js @@ -562,7 +562,7 @@ describe( 'upcastTable()', () => { } ); describe( 'headingRows', () => { - it( 'should be able to detect heding row in 2x2 table', () => { + it( 'should be able to detect heading row in 2x2 table', () => { editor.setData( '' + '' + @@ -590,7 +590,7 @@ describe( 'upcastTable()', () => { ); } ); - it( 'should be able to detect heding row in table with caption', () => { + it( 'should be able to detect heading row in table with caption', () => { editor.setData( '
' + '' + @@ -645,7 +645,7 @@ describe( 'upcastTable()', () => { ); } ); - it( 'should be able to detect heding row in 2x1 table', () => { + it( 'should be able to detect heading row in 2x1 table', () => { editor.setData( '
Concerts
' + '' + @@ -671,7 +671,7 @@ describe( 'upcastTable()', () => { ); } ); - it( 'should be able to detect heding row that has colspan', () => { + it( 'should be able to detect heading row that has colspan', () => { editor.setData( '
' + '' + @@ -700,6 +700,108 @@ describe( 'upcastTable()', () => { '
' ); } ); + + it( 'should not treat the row containing only th as a heading row if it follows rowspan=2', () => { + editor.setData( + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
000102
1011
20
303132
' + ); + + expectModel( + '' + + '' + + '00' + + '01' + + '02' + + '' + + '' + + '10' + + '11' + + '' + + '' + + '20' + + '' + + '' + + '30' + + '31' + + '32' + + '' + + '
' + ); + } ); + + it( 'should not treat the row containing only th as a heading row if it is last row of rowspan=3', () => { + editor.setData( + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
000102
1011
20
30
404142
' + ); + + expectModel( + '' + + '' + + '00' + + '01' + + '02' + + '' + + '' + + '10' + + '11' + + '' + + '' + + '20' + + '' + + '' + + '30' + + '' + + '' + + '40' + + '41' + + '42' + + '' + + '
' + ); + } ); } ); describe( 'block contents', () => {