Ticket #2389 (closed Bug: worksforme)

Opened 6 years ago

Last modified 5 years ago

Merge Cells removes attributes from tr in table

Reported by: mattleff Owned by:
Priority: Normal Milestone:
Component: General Version: FCKeditor 2.5 Beta
Keywords: Confirmed DataLoss Cc:

Description

When two cells are merged the class attribute on the tr(s) in the table is removed.

Steps to reproduce:

Click on the source button and remove the existing code and paste in the following

<table>
    <tbody>
        <tr class="sample_class">
            <td>Test cell</td>
            <td>Test cell 2</td>
        </tr>
    </tbody>
</table>

Click the source button again to exit source view and click in the two cells. Right-click and select merge cells (or merge right). Click source again and the class attribute is removed from the tr.

This is reproducible in either the demo on FCKeditor.net or the nightly build. It reproduces in FF3, FF2, IE7, and IE6 and doesn't seem to be dependent on platform used.

Attachments

2389.patch (3.1 KB) - added by shri046 6 years ago.
2389.2.patch (2.9 KB) - added by shri046 6 years ago.
Revised patch
2389_3.patch (685 bytes) - added by kwillems 6 years ago.

Change History

comment:1 Changed 6 years ago by alfonsoml

  • Keywords Confirmed added
  • Version changed from FCKeditor 2.6.2 to FCKeditor 2.5 Beta
  • Component changed from Core : Styles to General

The problem is the FCKTableHandler._InstallTableMap function that does removes all the rows in the table and then recreates them again, so any attribute in any row affected by the table operations (merge and split cells) will be lost.

It's not related to the Styles component, but to the tables handling, the code was introduced with [690], part of 2.5beta

comment:2 follow-up: ↓ 3 Changed 6 years ago by alfonsoml

  • Keywords DataLoss added
  • Summary changed from Merge Cells removes class from tr in table to Merge Cells removes attributes from tr in table

comment:3 in reply to: ↑ 2 Changed 6 years ago by shri046

Replying to alfonsoml:

Adding a patch for this defect. Tested it on FireFox 2.0 and IE 6 and seems to have fixed the issue.

Changed 6 years ago by shri046

Changed 6 years ago by shri046

Revised patch

comment:4 Changed 6 years ago by alfonsoml

Having a function named CopyAttributesToArray that returns an object looks really strange. The comments should reflect the functionality in a generic way (don't use "on the row element", or that "could probably be used in other scenarios")

To me it looks strange that the functions added to fckdomtools are asymetric. One takes care of doing all the loop, but the second one only applies one attribute at a time.

The logic about rowCount is flawed. You can test that merging some cells in this table changes the classes:

<table cellspacing="1" cellpadding="1" border="1" width="200">
    <tbody>
        <tr class="odd">
            <td>&nbsp;</td>
            <td>&nbsp;</td>
        </tr>
        <tr>
            <td>&nbsp;</td>
            <td>&nbsp;</td>
        </tr>
        <tr class="odd">
            <td>&nbsp;</td>
            <td>&nbsp;</td>
        </tr>
    </tbody>
</table>

Changed 6 years ago by kwillems

comment:5 Changed 6 years ago by kwillems

I added a litle patch to preserve classes on rows and so on.

Regards, Koen Willems

comment:6 Changed 6 years ago by kwillems

Oops ... please wait reviewing this patch. I stumbled upon some problems with another patch i'm making.

comment:7 Changed 6 years ago by kwillems

Ok, go ahead ... the patch is working!

comment:8 Changed 5 years ago by mattleff

  • Status changed from new to closed
  • Resolution set to worksforme

Closing as this WFM on the CKEditor trunk.

Note: See TracTickets for help on using tickets.
© 2003 – 2012 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy