Opened 4 years ago

Last modified 4 years ago

#14289 confirmed Bug

Issue with Merging and Deleting Cell at last column

Reported by: Bryan Foo Owned by:
Priority: Normal Milestone:
Component: Core : Tables Version:
Keywords: Cc:

Description

in cases where there are multiple row spans.

merging or deleting cell at the last column will cause the next row to move up to the right of the last column cell of the current row.

sample source code at "https://dl.dropboxusercontent.com/u/67609133/CKEditor%20Bug%20Source%20Code.htm"

Steps to reproduce

Merging Cell

  1. Copy sample source code to CKEditor
  2. Right click on the cell containing red text "Merge Cell Down"
  3. Select Cell > Merge Down

Deleting Cell

  1. Copy sample source code to CKEditor
  2. Right click on the cell containing red text "Delete This Cell"
  3. Select Cell > Delete Cells

Expected result

Merging Cell Cell will merge correctly with cell containing text "Row 5, Col 5". Rows should be maintain.

Deleting Cell Cell will be deleted. Rows should be maintain.

Actual result

Merging Cell Cell will merge but the next row will shift up to the right.

Deleting Cell Cell will be deleted but the next row will shift up to the right.

Other details (browser, OS, CKEditor version, installed plugins)

Browser: (Firefox 38, IE 8-11, Chrome 47) OS: (Win 8.1 Pro) CKEditor: (4.5.6) CKEditor Build: (http://ckeditor.com/builder/24cdab0854f59a615f60afcd3b55d623)

Change History (10)

comment:1 Changed 4 years ago by Jakub Ś

Status: newpending
Version: 4.5.6

There are couple of rowspan, colspan bugs in the editor. I must admit that.

I have been also able to reproduce this problem but I have some doubts this is a bug in the editor. I have been trying to fix the table structure for quite some time but failed to do so.

@blurdude are you able to fix the table structure after deleting or merging cells? I'm asking because it may be the problem with the table itself.

comment:2 Changed 4 years ago by Bryan Foo

After merging/deleting cell, I can manually adjust the rowspan. But that is because I understand how rowspan works.

For the layman, they wouldn't have any idea what was the cause and how to solve it. They would be expecting the editor to automatically adjust all relevant rowspan and/or colspan after doing a merger/deletion. It is even more difficult when it comes to knowledge transfer to the older folks.

While I understand that this may not be easy, but I feel it is necessary to automate this.

I have users, whom deal extensively with structuring contents using table, telling me that they WILL NOT use CKEditor if this problem isn't fixed.

comment:3 Changed 4 years ago by Jakub Ś

After merging/deleting cell, I can manually adjust the rowspan. But that is because I understand how rowspan works. For the layman, they wouldn't have any idea what was the cause and how to solve it.

Please treat me as such and show me which row/colspans should be adjusted. I'm asking because I have been trying to adjust these cells for 30 minutes and didn't managed to.
Perhaps I have missed something simple but currently I think it is not possible to manually adjust cells by just changing one or two rowspans/colspans here and there after table structure has been destroyed.

comment:4 Changed 4 years ago by Bryan Foo

ok here are the steps for recovering after merging the cells.

  1. Right click on the merged cell, select "cell properties", delete row span 2. Click ok.
  2. Right click on "Row 4, Col 4", select "cell properties", delete row span 2. Click ok.
  3. Right click on "Row 4, Col 3", select "cell properties", delete row span 2. Click ok.
  4. Right click on "Row 3, Col 2", select "cell properties", change row span from 3 to 2. Click ok.
  5. Right click on "Row 1, Col 1", select "cell properties", change row span from 8 to 7. Click ok.

Done.


Edit for Cell Delete:

  1. Remove rowspan 2 for "Row 6, Col 3"
  2. Remove rowspan 2 for "Row 6, Col 4"
Last edited 4 years ago by Jakub Ś (previous) (diff)

comment:5 Changed 4 years ago by Jakub Ś

While it makes sense from how the table looks point of view it will be very hard or impossible to implement it looking at the HTML. You need to go through all the cells to the right but how will you find cells to the right in HTML?

At first I thought that perhaps we could go through all rows and decrease row span which is bigger but this doesn't make sense.

I don't think it will be possible to implement it.

comment:6 Changed 4 years ago by Marek Lewandowski

Status: pendingconfirmed

Delete case

Technically to keep table correct you should also reduce "Row 6, Col 2" and "Row 1, Col 1" cells rowspan down by one.

Conclusion

Yes, current solution is buggy. As said before, the right solution will be in fact very hard to implement. It requires little bit of heuristics to make a good call.

A as a side note interesting fact that for this particular TC (deleting) you could just use "Delete rows" option, and it creates perfectly fine, expected HTML. :)

comment:7 in reply to:  5 ; Changed 4 years ago by Bryan Foo

Replying to j.swiderski:

While it makes sense from how the table looks point of view it will be very hard or impossible to implement it looking at the HTML. You need to go through all the cells to the right but how will you find cells to the right in HTML?

At first I thought that perhaps we could go through all rows and decrease row span which is bigger but this doesn't make sense.

I don't think it will be possible to implement it.

I had done some random test and it seems to affect only the last column

To be specific, only when merging down from Row 5, Col 5.

It is ok if I merge down from Row 3, Col 5. It is also ok if I merge down from Row 5, Col 5.

So we probably only need to adjust cells to the left.

Anyway, thank you for spending time on this, I do understand your point regarding implementing a proper solution.

But if something can be done, hopefully in the future, it will make the editor more user friendly. :)

comment:8 in reply to:  6 Changed 4 years ago by Bryan Foo

Replying to m.lewandowski:

Delete case

Technically to keep table correct you should also reduce "Row 6, Col 2" and "Row 1, Col 1" cells rowspan down by one.

Conclusion

Yes, current solution is buggy. As said before, the right solution will be in fact very hard to implement. It requires little bit of heuristics to make a good call.

A as a side note interesting fact that for this particular TC (deleting) you could just use "Delete rows" option, and it creates perfectly fine, expected HTML. :)

Hi,

understand your point of view as well regarding Delete case.

To avoid confusion, will it be better to phase out inserting and deleting cells? Not sure how it can be useful when a better way is to insert and delete row/column.

Just my 5cents worth for your consideration. :)

comment:9 in reply to:  7 Changed 4 years ago by Bryan Foo

Replying to blurdude:

I had done some random test and it seems to affect only the last column

after more testing, I think it affected the first column as well when merging. =(

comment:10 Changed 4 years ago by Jakub Ś

To avoid confusion, will it be better to phase out inserting and deleting cells? Not sure how it can be useful when a better way is to insert and delete row/column.

This is one of things to consider. I can't find the ticket at the moment but we had such suggestion already and in my personal opinion it makes sense as many table problems are caused by invalid rowspan/colspan and cell operations.

If on the other hand we will be able to re-implement tables as @m.lewandowski has written, perhaps there will be no need for removing options for cells.

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