Opened 8 years ago
Closed 8 years ago
#16675 closed Bug (fixed)
Copy formatting - tables issue
Reported by: | Wiktor Walc | Owned by: | Tomasz Jakut |
---|---|---|---|
Priority: | Normal | Milestone: | CKEditor 4.6.2 |
Component: | General | Version: | 4.6.0 |
Keywords: | Cc: |
Description (last modified by )
Steps to reproduce
-
<h2 style="text-align: center;"><span style="font-family:Arial,Helvetica,sans-serif">CONSOLIDATED STATEMENTS OF CASH FLOWS </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"> </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> </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> </td> <td> </td> </tr> <tr> <td>Acquisitions, net of cash acquired, and other</td> <td>84</td> <td>105</td> <td> </td> <td> </td> </tr> <tr> <td>Sales and maturities of marketable securities</td> <td>1,431</td> <td>1,045</td> <td> </td> <td> </td> </tr> <tr> <td>Purchases of marketable securities</td> <td>2,076</td> <td>1,122</td> <td> </td> <td> </td> </tr> <tr> <td>Net cash provided by (used in) investing activities</td> <td>2,570</td> <td>1,377</td> <td> </td> <td> </td>
- http://ckeditor.com/latest/samples/index.html
- 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)
Attachments (1)
Change History (14)
Changed 8 years ago by
Attachment: | copyformatting2.gif added |
---|
comment:1 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 8 years ago by
Milestone: | → CKEditor 4.6.1 |
---|---|
Version: | → 4.6.0 |
comment:3 Changed 8 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 8 years ago by
Milestone: | CKEditor 4.6.1 → CKEditor 4.6.2 |
---|---|
Status: | new → confirmed |
comment:5 Changed 8 years ago by
Owner: | set to Tomasz Jakut |
---|---|
Status: | confirmed → assigned |
comment:6 Changed 8 years ago by
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.
comment:7 Changed 8 years ago by
Status: | assigned → review |
---|
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.
comment:8 Changed 8 years ago by
Status: | review → review_failed |
---|
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?
comment:9 Changed 8 years ago by
Status: | review_failed → review |
---|
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.
comment:10 Changed 8 years ago by
Status: | review → review_failed |
---|
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
.
comment:11 Changed 8 years ago by
Status: | review_failed → review |
---|
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…).
comment:13 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Merged to master
with 851352c.
Confirmed, however we won't be able to contain it within 4.6.1 - looking for 4.6.2.