Ticket #2287 (closed Bug: fixed)

Opened 6 years ago

Last modified 6 years ago

Bad Spacing between two tables (FF2 Only)

Reported by: dshafik Owned by: fredck
Priority: Normal Milestone: FCKeditor 2.6.2
Component: General Version: SVN (FCKeditor) - Retired
Keywords: Confirmed Firefox Review+ Cc:

Description

When you insert a table, click to the right side of it, and then add in a second table, it will display something like:

<table>...</table>

<table>...</table>

within the editor, but when you preview (or after saving) it shows up like:

<table>...</table>


<table>...</table>

The reason is that between the two tables, in the editor, there is a <p/> tag, which FF does not render. But when you preview/save, it is transformed into <p></p>.

You can see this in action at: http://www.screencast.com/users/DShafik/folders/Jing/media/d4769677-866b-4a5b-8d21-1aa0a31cd14c

This issue affects 2.5.1 (at least) through to, and including trunk

Attachments

table_spacing_empty_paragraph.patch (761 bytes) - added by dshafik 6 years ago.
Table Spacing Patch
2287.patch (833 bytes) - added by martinkou 6 years ago.
2287_2.patch (3.0 KB) - added by fredck 6 years ago.

Change History

Changed 6 years ago by dshafik

Table Spacing Patch

comment:1 Changed 6 years ago by fredck

  • Keywords Confirmed Firefox added
  • Version set to SVN

Confirmed with FF2. FF3 is ok.

Leave it targeted to the 2.6.2, to check the patch. Thanks dshafik!

comment:2 Changed 6 years ago by martinkou

The patch works and makes sense. Good job. :)

I've made some simplifications to it and some coding style fixes, and added a comment to the code. Proposing a new patch for review.

Changed 6 years ago by martinkou

comment:3 Changed 6 years ago by martinkou

  • Keywords Review? added

comment:4 Changed 6 years ago by fredck

  • Owner set to fredck
  • Status changed from new to assigned
  • Keywords Review- added; Review? removed

The path is ok. It actually fixes FixBlock correctly, and should definitely land into the trunk.

But, other than the missing changelog entry (:P), one thing that bugs me out is: "Why the hell we have that <p/> there? Who said it should be there? After all, I want a table there!".

I'm working on a fix for all this, which will include the proposed patch.

Changed 6 years ago by fredck

comment:5 Changed 6 years ago by fredck

  • Keywords Review+ added; Review- removed

This new patch includes the original proposed fix, but includes also another fix for the insertion of block elements. The following is the proposed behavior:

  • If the selection is not inside a block (the source of the problem in this ticket), nothing in special is done. The new block is simply inserted at the current position. There is no need to SplitBlock (and therefore "FixBlock") first.
  • If the selection is at the start of a block, the new block is simply insert "before" that block.
  • If the selection is at the end of a block, the new block is then insert "after" that block.
  • If the selection is at the middle of the block, the previous behavior is taken and the SplitBlock is called.

comment:6 Changed 6 years ago by fredck

  • Keywords Review? added; Review+ removed

:| I always do that.

comment:7 Changed 6 years ago by martinkou

  • Keywords Review+ added; Review? removed

comment:8 Changed 6 years ago by fredck

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

Fixed with [2106].

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