Copy/paste table in Firefox results in just first cell being pasted

[Jakub Ś]

Steps to reproduce

1. Paste below code in source mode and switch to wysiwyg

   <table border="1" cellpadding="1" cellspacing="1" style="width:500px">
       <tbody>
           <tr>
               <td style="background-color: rgb(102, 102, 204);">s</td>
               <td>s</td>
           </tr>
           <tr>
               <td>s</td>
               <td style="background-color: rgb(255, 255, 0);">s</td>
           </tr>
           <tr>
               <td style="background-color: rgb(255, 204, 0);">s</td>
               <td style="background-color: rgb(255, 204, 0);">s</td>
           </tr>
       </tbody>
   </table>
   

2. Select whole table with a mouse and Press Ctrl+C (You can also use Copy from context menu)
3. Press Ctrl+V or right-click below table and select Paste (Ctrl+V into paste dialog and click OK)

Expected result

Whole table should be pasted and formatting preserved.

Actual result

Only first row gets pasted.

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

Problem can be reproduced from CKEditor 4.5.0 in Firefox only.

NOTE: From CKEditor 4.0 - 4.4.8 the above scenario resulted in table being pasted but without width style (background was preserved).

[Jakub Ś]

Temporary workaround is to select table element by clicking table element on element's path. This feature selects whole table and using later the Ctrl+C/Ctrl+V or context menu works as expected.

[Marek Lewandowski]

It possibly can be related with #13883.

[Piotr Jasiun]

This is because in 4.5.0 custom copy and paste was introduced and it does not handle multi-range selection, it use only the first range. Firefox use multiple ranges to represent table selection - each row is a separate range. This is why only the first row is copied.

To make works custom copy and paste should handle multi-ranges.

[Marek Lewandowski]

This issue is likely to originate from ​editor.getSelectedHtml which relies only on a single range. From clipbaord perspective it's called in ​CKEDITOR.plugins.clipboard.dataTransfer method.

[Marek Lewandowski]

Pushed tests to ​t/13884.

[Tomasz Jakut]

Changing editor.getSelectedHtml to handle multiple ranges seems to fix the problem.

I've created new private function ​createDocFragmentFromRanges which can be used to handle all cases of creating DocumentFragments from mutliple ranges (at the moment it handles only tables in Firefox).

Pushed ​branch:t/13884.

[Marek Lewandowski]

Rebased to master and added a another TC in partialtable.md.

• Partial copy does not necessarily works well.

This fix is a little too simple. Ranges does not need to be set from the very beginning of the selection all the way down to the last element in the selection. It might contain some parts of the document (table in this case) while skipping the other parts.

If you select 2x2 cells part out of a 3x3 table then the whole table gets pasted. Before multirange was broken in 4.5.0 this case was working OK (e.g. see 4.3 tag).

I've ​added a TC for that.

• Please create an unit that will automate test added in ​new TC commit.

• ​editor.js - use ​`CKEDITOR.dom.element#getName` instead

[Tomasz Jakut]

I've updated createDocumentFragmentFromRanges function. Now it iterates through all ranges and recreates the copied table from scratch. I've also added unit test for partial table copy. Pushed changes to ​branch:t/13884.

[Marek Lewandowski]

Fixed with ​git:8a5e2265aa (merged to master).

Few additional comments:

Proposed fix does the job well, but what I'd like to see here is a better integration with already existing code. All the heavy operations are well ​hidden behind this intefrace.

New code duplicates a little code from getHtmlFromRangeHelpers.tree.rebuild. Ofc the problem with all these methods is that they're focused on a single range.

I think we could change this piece of code:

var that = {
	doc: this.getDocument(),
	// Leave original range object untouched.
	range: range.clone()
};

We could simply add either selection there or simply ranges array, so that each (like getHtmlFromRangeHelpers.tree.rebuild) can be aware that it's a multirange call.

For now we'll merge this code as it's a very important fix that can no longer wait, but there will be a follow up ticket.