Copy formatting - tables issue

[Wiktor Walc]

Steps to reproduce

1. <h2 style="text-align: center;"><span style="font-family:Arial,Helvetica,sans-serif">CONSOLIDATED STATEMENTS OF CASH FLOWS&nbsp;</span></h2>
   
   <p style="text-align: center;"><span style="font-family:Arial,Helvetica,sans-serif">(in millions of $)</span></p>
   
   <table cellpadding="3" cellspacing="1" style="height:80px; width:800px">
   	<tbody>
   		<tr>
   			<td style="background-color:#eeeeee; text-align:center; width:436px"><span style="font-size:14px"><span style="font-family:Arial,Helvetica,sans-serif"><strong>INVESTING ACTIVITIES</strong></span></span></td>
   			<td style="background-color:#eeeeee; text-align:center; width:96px"><strong><span style="font-size:14px"><span style="font-family:Arial,Helvetica,sans-serif">Q1 2015</span></span></strong></td>
   			<td style="background-color:#eeeeee; text-align:center; width:95px"><span style="font-size:14px"><span style="font-family:Arial,Helvetica,sans-serif"><strong>Q2 2015</strong></span></span></td>
   			<td style="background-color:#eeeeee; text-align:center; width:92px"><span style="font-size:14px"><span style="font-family:Arial,Helvetica,sans-serif"><strong>Q3 2015</strong></span></span></td>
   			<td style="background-color:#eeeeee; text-align:center; width:92px"><span style="font-size:14px"><span style="font-family:Arial,Helvetica,sans-serif">Q4 <span style="display:none">&nbsp;</span><strong>2015</strong></span></span></td>
   		</tr>
   		<tr>
   			<td style="width:436px">Purchases of property and equipment</td>
   			<td style="width:96px">1,841</td>
   			<td style="width:95px">1,195</td>
   			<td style="width:92px">1,456</td>
   			<td style="width:89px">1,980</td>
   		</tr>
   		<tr>
   			<td style="width:436px">Acquisitions, net of cash acquired, and other</td>
   			<td style="width:96px">84</td>
   			<td style="width:95px">105</td>
   			<td style="width:92px">87</td>
   			<td style="width:89px">107</td>
   		</tr>
   		<tr>
   			<td style="width:436px">Sales and maturities of marketable securities</td>
   			<td style="width:96px">1,431</td>
   			<td style="width:95px">1,045</td>
   			<td style="width:92px">1,765</td>
   			<td style="width:89px">1,823</td>
   		</tr>
   		<tr>
   			<td style="width:436px">Purchases of marketable securities</td>
   			<td style="width:96px">2,076</td>
   			<td style="width:95px">1,122</td>
   			<td style="width:92px">1,587</td>
   			<td style="width:89px">1,290</td>
   		</tr>
   		<tr>
   			<td style="width:436px">Net cash provided by (used in) investing activities</td>
   			<td style="width:96px">2,570</td>
   			<td style="width:95px">1,377</td>
   			<td style="width:92px">987</td>
   			<td style="width:89px">1,560</td>
   		</tr>
   	</tbody>
   </table>
   
   <p>&nbsp;</p>
   
   <table cellpadding="3" cellspacing="1">
   	<tbody>
   		<tr>
   			<td>INVESTING ACTIVITIES</td>
   			<td>Q1 2016</td>
   			<td>Q2 2016</td>
   			<td>Q3 2016</td>
   			<td>Q4 2016</td>
   		</tr>
   		<tr>
   			<td>Purchases of property and equipment</td>
   			<td>1,841</td>
   			<td>1,195</td>
   			<td>&nbsp;</td>
   			<td>&nbsp;</td>
   		</tr>
   		<tr>
   			<td>Acquisitions, net of cash acquired, and other</td>
   			<td>84</td>
   			<td>105</td>
   			<td>&nbsp;</td>
   			<td>&nbsp;</td>
   		</tr>
   		<tr>
   			<td>Sales and maturities of marketable securities</td>
   			<td>1,431</td>
   			<td>1,045</td>
   			<td>&nbsp;</td>
   			<td>&nbsp;</td>
   		</tr>
   		<tr>
   			<td>Purchases of marketable securities</td>
   			<td>2,076</td>
   			<td>1,122</td>
   			<td>&nbsp;</td>
   			<td>&nbsp;</td>
   		</tr>
   		<tr>
   			<td>Net cash provided by (used in) investing activities</td>
   			<td>2,570</td>
   			<td>1,377</td>
   			<td>&nbsp;</td>
   			<td>&nbsp;</td>
   

2. ​http://ckeditor.com/latest/samples/index.html
3. Paste the code in source mode and follow the gif (copy formatting from Q3 to Q4)

Expected result

Actual result

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

[Marek Lewandowski]

Confirmed, however we won't be able to contain it within 4.6.1 - looking for 4.6.2.

[Tomasz Jakut]

The issue seems to be connected with the way in which styles are applied inside the editor. They're applied from the text node to the top of DOM tree. In rare cases (such as this one) it causes the change of the whole selection and breaks the walker inside ​getNodeAndApplyCmd function.

[Tomasz Jakut]

Pushed proposed solution to ​branch:t/16675.

The solution is simple: I've added additional check to the getNodeAndAppyCmd function that checks if the range has correct end container. If it's incorrect, the whole function is restricted to the first matching element.

[kkrzton]

Generally the fix works fine and solves the issue.

However, I have some doubts which I would like to be clarified. If I understand correctly, in this case the range which is used has incorrect end container. Such range is created while applying styles (but before any changes in a DOM) on the selected element. For the table context, first, styles on the text context are applied (all styling elements are removed and new/copied ones are applied) and then styles on the table context are applied. The same range (from applyFormatting event) is used for applying styles in both contexts, however, after first styles were applied the range is already outdated (the styles elements it was based on were removed by style application on text context).

In this case, the issue is somehow caused by design (the flow/order of operations). If we are okay with that, the proposed solution should be sufficient (but we need to make sure if there are no similar, uncovered cases with different elements like e.g. lists). The other solution would be to adjust the way the styles are applied but it would require much more work probably. WDYT @t.jakut?

[Tomasz Jakut]

Yes, it could be said that the defect is caused by the design of our mechanism for applying styles. The current fix covers also lists (as they are also handled by getNodeAndApplyCmd. I've added unit test for them, too.

BTW I've just discovered alternative method: bookmarks (just like it's done for lists). I've pushed it to the ​branch:t/16675-b. I think that this new approach is less dirty and should cover all situations that could be caused by our style system.

[kkrzton]

The branch:t/16675-b is the way to go IMHO. Much cleaner solution and it adds a proper fix (updating the range when it has changed).

However, some unit tests are failing on FF, Safari, IE8, IE11 and Edge:

test applying styles from one cell to other (classic)
test applying styles from one cell to other (inline)

Btw. I think the test name should be test applying styles from one cell to another instead of test applying styles from one cell to other.

[Tomasz Jakut]

I've updated test and pushed to ​branch:t/16675-b. I've abandoned approach with comparing content of the editor in favor of just checking if the last cell got styles or not (no more problems with incompatibilities between browsers…).

[kkrzton]

All tests passing, LGTM!

[kkrzton]

Merged to master with ​851352c.