Ticket #2486 (closed Bug: fixed)

Opened 4 months ago

Last modified 7 weeks ago

Vertically splitting cell with colspan > 1 breaks table layout

Reported by: shri046 Owned by: alfonsoml
Priority: Normal Milestone: FCKeditor 2.6.4
Component: General Version: FCKeditor 2.6.3
Keywords: Review+ Cc: fredck

Description (last modified by alfonsoml) (diff)

Tested below steps on demo page (v2.6.3) in FF and IE

  1. Create a 3x2 table.
  2. Split cell 1 in row 1 horizontally.
  3. Split cell 1 in row 2 vertically.

Result: The cell is split vertically into 2 but only the first cell has colspan="2". The second cell is missing this colspan which breaks the table layout.

Attachments

2486.patch (2.2 KB) - added by shri046 3 months ago.
Revised patch
2486_1.patch (2.3 KB) - added by shri046 3 months ago.

Change History

Changed 3 months ago by alfonsoml

  • keywords HasPatch added
  • summary changed from Vertically splitting cell with colspan > 2 breaks table layout to Vertically splitting cell with colspan > 1 breaks table layout

I've fixed the title because I guess that the problem is not only with colspan>2, but any colspan>1

That fix looks simple, so it's nice :D

The change in the first line is due to the UTF-8 encoding, can you fix it please?
And can you add an entry to the What's new file and include it in the patch?
And when you add a patch add the HasPatch keyword (or Review? if you really think that it's ready)

Changed 3 months ago by shri046

Revised patch

Changed 3 months ago by shri046

  • keywords Review? added; HasPatch removed

Ok I have attached a revised patch and added the change to the What's new file as well. Not sure what is with all the encoding stuff but what should be the normal encoding used for these files? Because the first time I did use UTF-8 encoding and I normally use TextPad for editing files.

Changed 3 months ago by shri046

Changed 3 months ago by shri046

Minor modification. Need to set colSpan only if it is greater than 1.

Changed 8 weeks ago by alfonsoml

  • cc fredck added
  • keywords Review+ added; Review? removed

The patch is OK, (only minor issue is the alignment in the what's new file that should be fixed before commiting)

Fred, can he commit the patch or should I take care of it? (or does anyone else takes care?)

Changed 7 weeks ago by fredck

  • milestone set to FCKeditor 2.6.4

alfonsoml, shri046 is not a committer. This ticket should have been marked as HasPatch instead of Review?. But, you have already reviewed it, so you should be satisfied with it.

At this point, please take the ownership of the ticket and go ahead committing it. Thanks!

Changed 7 weeks ago by alfonsoml

  • owner set to alfonsoml
  • description modified (diff)

I think that it's interesting that even someone isn't a committer he might mark a patch requesting for review. Usually a HasPatch means just a broad idea, or a whole file attached, but it's clear that requesting for review means that he really wants to push the ticket to the end, and if everything goes well he might keep on contributing and providing useful patches.

Changed 7 weeks ago by alfonsoml

  • status changed from new to closed
  • resolution set to fixed

Fixed with [2571]

Note: See TracTickets for help on using tickets.