Opened 11 years ago

Closed 8 years ago

#9944 closed Bug (duplicate)

insertcellbefore and insertcellafter in nested tables is incorrect

Reported by: Michiel Overeem Owned by:
Priority: Normal Milestone:
Component: Core : Tables Version: 3.0
Keywords: Cc:

Description

The insertcellbefore and insertcellafter commands from the tabletools plugin is incorrect. The insertCell function found in this plugin looks for the first tablecell (td) ascendant. If there is none, it looks for the first table-header-cell (th) ascendant. It does not check if the cell is a child of the current table. If the tables are nested, or the editor itself is nested inside of a table, the cell is added to the incorrect table. I've attached a corrected function that can be used to patch this bug.

Attachments (2)

tabletools_insertcell.js (763 bytes) - added by Michiel Overeem 11 years ago.
corrected insertCell function
tabinsertcellproblem.html (1021 bytes) - added by Michiel Overeem 11 years ago.
Sample file to illustrate the problem

Download all attachments as: .zip

Change History (8)

Changed 11 years ago by Michiel Overeem

Attachment: tabletools_insertcell.js added

corrected insertCell function

Changed 11 years ago by Michiel Overeem

Attachment: tabinsertcellproblem.html added

Sample file to illustrate the problem

comment:1 Changed 11 years ago by Michiel Overeem

Attached is a sample file to show the problem. Inserting a cell from the contextmenu in the first cell ('header one') inserts the cell in the outer table, which is not part of the contenteditable element. Inserting in the cells on row two ('cell one' and 'cell two') works correct.

comment:2 Changed 11 years ago by Michiel Overeem

The same problem is present in the getSelectedCells function inside the plugin.

comment:3 Changed 11 years ago by Jakub Ś

Keywords: tabletools insertcell removed
Status: newconfirmed
Version: 4.0.13.0
  1. Problem can be reproduced from CKEditor 3.0 in both 3.x and 4.x.
  2. Just put cursor in header cell and select insert cell before or after

Result: Cell will be inserted in outer table

The same thing can be reproduced with iframed editor with below code:

<table border="1" cellpadding="1" cellspacing="1" style="width: 500px;">
	<tbody>
		<tr>
			<td>
				&nbsp;</td>
			<td>
				<p>
					&nbsp;</p>
				<p>
					&nbsp;</p>
				<table border="1" cellpadding="1" cellspacing="1" style="width: 500px;">
					<thead>
						<tr>
							<th scope="col">
								&nbsp;</th>
							<th scope="col">
								&nbsp;</th>
						</tr>
					</thead>
					<tbody>
						<tr>
							<td>
								&nbsp;</td>
							<td>
								&nbsp;</td>
						</tr>
					</tbody>
				</table>
				<p>
					&nbsp;</p>
			</td>
			<td>
				&nbsp;</td>
		</tr>
	</tbody>
</table>

comment:4 Changed 11 years ago by Jakub Ś

@michielovereem - To get your proposed fix tested and applied (if it is correct) please fork CKEditor 4 code from https://github.com/ckeditor/ckeditor-dev, prepare your fix and make a "pull request" (here is the example list of pull requests https://github.com/ckeditor/ckeditor-dev/pulls?direction=desc&page=1&sort=created&state=open). In comment please specify link to bug for which this fix was prepared. This is much faster way to get proposed fix implemented/rejected (if they are invalid). At least in V4.

In CKEditor 3.x we go through proposed SVN patches every couple of weeks (sometimes moths) and decide whether to apply them or not. This of course takes more time then pull requests on github but this is what versioning system offered and we had to adjust to it.

comment:5 Changed 11 years ago by Michiel Overeem

Added pull request with the required fix: https://github.com/ckeditor/ckeditor-dev/pull/36

comment:6 Changed 8 years ago by Jakub Ś

Resolution: duplicate
Status: confirmedclosed

This is a duplicate of #11192. @michielovereem I will move your pull request there.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy