Ticket #2389 (closed Bug: worksforme)

Opened 20 months ago

Last modified 2 months 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 Download (3.1 KB) - added by shri046 19 months ago.
2389.2.patch Download (2.9 KB) - added by shri046 19 months ago.
Revised patch
2389_3.patch Download (0.7 KB) - added by kwillems 14 months ago.

Change History

  Changed 20 months 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

follow-up: ↓ 3   Changed 20 months 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

in reply to: ↑ 2   Changed 19 months 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 19 months ago by shri046

Changed 19 months ago by shri046

Revised patch

  Changed 19 months 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 14 months ago by kwillems

  Changed 14 months ago by kwillems

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

Regards, Koen Willems

  Changed 14 months ago by kwillems

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

  Changed 14 months ago by kwillems

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

  Changed 2 months 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.