Opened 8 years ago

Closed 8 years ago

#2486 closed Bug (fixed)

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)

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 (2)

2486.patch (2.2 KB) - added by shri046 8 years ago.
Revised patch
2486_1.patch (2.3 KB) - added by shri046 8 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 8 years 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 8 years ago by shri046

Revised patch

comment:2 Changed 8 years 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 8 years ago by shri046

comment:3 Changed 8 years ago by shri046

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

comment:4 Changed 8 years 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?)

comment:5 Changed 8 years 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!

comment:6 Changed 8 years ago by alfonsoml

  • Description modified (diff)
  • Owner set to alfonsoml

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.

comment:7 Changed 8 years ago by alfonsoml

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

Fixed with [2571]

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