T/13729 Table: Insert Column before and After fails if a cell vertical split exist #592
Conversation
| @bender-ckeditor-plugins: wysiwygarea, toolbar, table, tabletools, contextmenu | ||
|
|
||
| 1. Make a vertical split in the cell 2,2. | ||
| 2. Right click in the first cell in 4. column to insert a new column before it. |
There was a problem hiding this comment.
Column inserted from cell 4,2 and 4,3 is inserted in the right place (between 3rd and 4th column). However, if you right click in the 4,1 cell and insert column it is placed on the beginning of the table as a first column.
There was a problem hiding this comment.
Well, it even happens without splitting any cell (step 1).
There was a problem hiding this comment.
Happens only in Chrome 58+ (OS X).
There was a problem hiding this comment.
It's reproducible also in Safari. Additionally it's reproducible only if the selection is placed at the end of the cell. It seems to be connected with known issue in Webkit/Blink (it's nicely visible with tableselection plugin enabled; in such case cells 4.1 and 1.2 are selected after inserting new column). Probably very similar workaround as proposed for #577 could solve the issue. However this issue is not connected with the original one, so I'd move it to the dedicated issue.
| @@ -227,14 +227,17 @@ | |||
| return null; | |||
| } | |||
|
|
|||
| function getCellColIndex( cell, isStart ) { | |||
| function getCellColIndex( cell ) { | |||
There was a problem hiding this comment.
In the meantime, tableselection plugin have been created which also utilizes this function (passing isStart=true) in two places. The code there should be also adjusted. All plugins/table* unit tests are still passing which looks promising.
| 1. Make a vertical split in the cell 2,2. | ||
| 2. Right click in the first cell in 4. column to insert a new column before it. | ||
|
|
||
| **Epected result:** A new column is inserted between 3rd and 4th column. |
| @@ -45,6 +45,9 @@ | |||
| this.doTest( 'add-col-before-4', 'columnInsertBefore' ); | |||
| this.doTest( 'add-col-before-multi', 'columnInsertBefore' ); | |||
| this.doTest( 'add-col-before-multi2', 'columnInsertBefore' ); | |||
|
|
|||
| // (#13729) | |||
There was a problem hiding this comment.
We could refer here to GH issue which is #591.
| @@ -0,0 +1,8 @@ | |||
| @bender-tags: 4.5.7, tc, tabletools, 13729 | |||
There was a problem hiding this comment.
Please update tags here (CKEditor release tag and issue tag: 519, trac13729)
What is the purpose of this pull request?
Bug fix
Does your PR contain necessary tests?
All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.
This PR contains
What changes did you make?
Closes #591.