id	summary	reporter	owner	description	type	status	priority	milestone	component	version	resolution	keywords	cc
8675	Deleting cells in nested table removes outer table cell.	David J. Hamilton	Olek Nowodziński	"'''STEPS TO REPRODUCE'''

0.  Use either Chrome or Firefox.  Bug is likely reproducible in IE as well, but I haven't confirmed this.
1.  Navigate to a 3.6.2 cke sample page.  Presently, http://ckeditor.com/demo exhibits this error.
2.  Clear any existing content in the editor.
3.  Create a table with 3 rows and 2 columns (OUTER).
4.  In the top left cell, create a nested table with 3 rows and 2 columns (INNER).
5.  In the INNER table, select the top two cells.
6.  Right click to get the context menu and select cell -> delete cells.

'''EXPECTED RESULT'''

Only the top two cells of the INNER table are deleted.


'''ACTUAL RESULT'''

The top left cell of the OUTER table is removed, thus removing the entire INNER table.


'''MORE INFO'''

Please see related ticket #6289.  This seems to be a regression from sometime after 3.4.2.  Note that #6289 was reported as an IE-only issue, but this ticket affects FF and Chrome (and likely IE as well).


'''DIAGNOSIS'''

The two bits of relevant code are in the following files:
  http://dev.ckeditor.com/browser/CKEditor/trunk/_source/plugins/tabletools/plugin.js
  http://dev.ckeditor.com/browser/CKEditor/trunk/_source/core/dom/walker.js

In tabletools, the function getSelectedCells is of interest, especially lines 63-68.

{{{
54	while ( ( node = walker.next() ) )
55	{
56	  // If may be possible for us to have a range like this:
57	  // <td>^1</td><td>^2</td>
58	  // The 2nd td shouldn't be included.
59	  //
60	  // So we have to take care to include a td we've entered only when we've
61	  // walked into its children.
62	
63	  var parent = node.getAscendant( 'td' ) || node.getAscendant( 'th' );
64	  if ( parent && !parent.getCustomData( 'selected_cell' ) )
65	  {
66	    CKEDITOR.dom.element.setMarker( database, parent, 'selected_cell', true );
67	    retval.push( parent );
68	  }
69	}
}}}

This loop examines each node from the range start to the range end, according to walker.js.  I am not entirely clear on the semantics of walker.js so I cannot say for sure whether it is correct, but from tracing it seems to be doing something reasonable.

When one selects two cells, the relevant portion of the DOM looks something like this:


{{{
TABLE (outer)
  TR
    TD (call this cell OA)
      BR
      TABLE (inner)
        TR
          TD (call this cell IA)
            BR  (call this node IABR)
          TD (call this cell IB)
            BR  (call this node IBBR)
        TR
          TD (call this cell IC)
            BR  (call this node ICBR)
          TD
            BR
        TR
          TD
            BR
          TD
            BR
    TD
       BR
   TR
     TD
       BR
     TD
       BR
   TR
     TD
       BR
     TD
       BR
}}}

When we select the top two cells of the INNER table our range goes from IABR to IBBR.  It is also ''very'' easy to accidentally select the first cell of the next row, resulting in a range that goes from IABR to ICBR.  The walker considers these nodes, but also the table cells IA, IC and their parent TR.

Note that line 63 fails to pass true (i.e. includeself) to getAscendant, and so erroneously includes cell OA in retval.


'''PROGNOSIS/PATCH'''

Please find attached a patch for this problem.  This patch does two things.

  1.  Passes includeself for the two calls to getAscendant.
  2.  Ignores a number of table related nodes (TABLE TR TBODY TFOOT THEAD).  If the walker were to consider a TBODY element in a nested table, I think the same behaviour would happen without this check.

This patch was made against cke 3.6.2.


'''OTHER'''

If anything is unclear about the issue or patch, please do not hesitate to email me at either dhamilton at verticalresponse.com or cke at hjdivad.com.  Thank you in advance for your time."	Bug	closed	Normal	CKEditor 4.0.1	Core : Tables	3.6.1	fixed	HasPatch