Merging header column cells and body cells results in changed header column row

[aStewartDesign]

📝 Provide detailed reproduction steps

1. Insert a CKEditor table; add extra columns/rows
2. Make a column a header column
3. Merge a header column cell with a body cell

✔️ Expected result

Probably should not be able to merge header column cells with body column cells

❌ Actual result

The body cell becomes a header column cell however when the data model is reloaded, the entire column the merged cell is in becomes a header column. Taking the ultimate table of coffee drinks as an example: https://ckeditor.com/docs/ckeditor5/latest/features/table.html

1. Merge the "Latte" cell with "yes" from the "Espresso" column.
2. "Latte yes" cell is now a header cell; in the editor model, headerColumns is now "2" however the rest of the "Espresso" column is still visually not a header column.
3. If you re-load this table's data, it loads with headerColumns === 2 and visually shows this with the rest of the "Espresso" column showing as a header column; inconsistent and different from the visual data that was there before.

Maybe disabling merges between body/header column cells isn't the way to go but 1) it prevents this inconsistency in the way headerColumns are displayed 2) it keeps the merge behavior consistent with header rows (which already cannot merge with body cells).

I will have a PR with this fix available soon.

📃 Other details

• Browser: chrome/firefox
• OS: OSX
• CKEditor version: 5/latest
• Installed CKEditor plugins: …

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[jodator]

Hi @aStewartDesign! Thanks for the PR!.

Definitely there's a problem with this behavior as we normalize table header column so it will always be rectangular. This is because we use some simple heuristic to detect heading columns. This has it's own issues when it comes to detecting heading rows - when there's no <thead>.

As for the feature of merging cells outside the heading section, it does make sense to not allow this for columns as in rows to match the behavior. However, there is more to be done to match the expected behavior:

• ☑️ The MergeCellCommand - as in Disable horizontal merge between header and body cells. ckeditor5-table#225
• 🔲 The SetHeaderColumnCommand should match behavior of SetHeaderRowCommand also.

---

@Reinmar - we should decide whether we want to change this behavior in the first place.

I'm leaning towards it to match the behavior of the upcast conversion algorithm.

If we do - then we can review closely linked PR and add a follow up for the SetHeaderColumnCommand to match its behavior.

[Reinmar]

I'm leaning towards it to match the behavior of the upcast conversion algorithm.

Yeah, me too. Ideally, the user should not create invalid content in the editor and content which we cannot load later on seems to be invalid. There's also an accessibility aspect of allowing doing weird things with header cols/rows: #5584. So, taken that, I'm for prohibiting merging header cells beyond their respecitve header rows/cols.

[aStewartDesign]

Thanks @jodator and @Reinmar for the feedback! If you want, I'm happy to help update the behavior of SetHeaderColumnCommand. Just to verify, it looks like the changes there involve adding getOverlappingCells and splitVertically methods which are then called from the model.change() block in the execute method. If this seems right, I can go ahead and get another PR in. If it is more complicated than that, I can leave it for someone who is more familiar with the project.

[jodator]

If it is more complicated than that, I can leave it for someone who is more familiar with the project.

I don't recall exactly but you might be right. If you have time and willingnes then you can try :) If you get stuck just let us know. We want to update the SetHeaderCommand also but we can't do it in current iteration so your input is more then welcome :)

I've created a follow-up ticker for this: #5761.