Opened 6 years ago

Last modified 5 years ago

#11768 confirmed Bug

Merging table-cells using the cell-properties dialog is completely broken

Reported by: Eugen Owned by:
Priority: Normal Milestone:
Component: General Version: 3.0
Keywords: Cc:

Description

Reproduce by:

  1. http://ckeditor.com/demo#standard
  2. insert a 3x3 table
  3. select cel 1x1
  4. right click -> cell -> cell properties
  5. input 2 in "merge row cells"
  6. submit the dialog

Problem: Everytime you do this, no matter which values you choose for col/rowspan, the complete table-layout is broken, respectivly an extra broken column is inserted every time.

Change History (6)

comment:2 Changed 6 years ago by Eugen

After looking at it for a while, the issue isnt too complicated. When selecting a cell and setting either row or colspanning, lets say rowspanning, then only the attribute, lets say rowspan="3" is applied to the selected cell, but no TDs are removed or rather "merged" (their contents) - leaving the table with to many TDs. so its just not working the same way as visually selecting cells and then use "merge"

Last edited 6 years ago by Eugen (previous) (diff)

comment:3 Changed 6 years ago by Eugen

Summary: row/colspanning using dialog completely brokwnMergin cells using the cell-properties dialog completely broken

After digging even more into the code, its clear that the case decribed before is right: https://github.com/ckeditor/ckeditor-dev/blob/master/plugins/tabletools/dialogs/tableCell.js#L323 So only the property is set, no logic at all.

The odd thing is, that tabeltools implements a proper cell-merge method which is used when selecting cells and use the context-menu to "merge": https://github.com/ckeditor/ckeditor-dev/blob/master/plugins/tabletools/plugin.js#L449

So we just have to delegate it, thats it.

In the end, it would be better to remove the option completely here, but due to the bug that you cannot select cells with the IE #8830 there is yet not other option to merge cells with the IE.

comment:4 Changed 6 years ago by Eugen

Summary: Mergin cells using the cell-properties dialog completely brokenMerging table-cells using the cell-properties dialog is completely broken

comment:5 Changed 6 years ago by Eugen

i fix it in a fork but beeing honest, the amount of issues, edge cases and bugs in this code .. i would rather suggest to completly remove spanning support at all :)

comment:6 Changed 5 years ago by Jakub Ś

Status: newconfirmed
Version: 4.3.43.0

This issue is more general and we have many cases like this reported, I however like the little research made here thus I'm confirming this ticket.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy