Opened 9 years ago

Closed 9 years ago

#2287 closed Bug (fixed)

Bad Spacing between two tables (FF2 Only)

Reported by: Davey Shafik Owned by: Frederico Caldeira Knabben
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 (3)

table_spacing_empty_paragraph.patch (761 bytes) - added by Davey Shafik 9 years ago.
Table Spacing Patch
2287.patch (833 bytes) - added by Martin Kou 9 years ago.
2287_2.patch (3.0 KB) - added by Frederico Caldeira Knabben 9 years ago.

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by Davey Shafik

Table Spacing Patch

comment:1 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Confirmed Firefox added
Version: SVN

Confirmed with FF2. FF3 is ok.

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

comment:2 Changed 9 years ago by Martin Kou

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 9 years ago by Martin Kou

Attachment: 2287.patch added

comment:3 Changed 9 years ago by Martin Kou

Keywords: Review? added

comment:4 Changed 9 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed
Owner: set to Frederico Caldeira Knabben
Status: newassigned

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 9 years ago by Frederico Caldeira Knabben

Attachment: 2287_2.patch added

comment:5 Changed 9 years ago by Frederico Caldeira Knabben

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 9 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review+ removed

:| I always do that.

comment:7 Changed 9 years ago by Martin Kou

Keywords: Review+ added; Review? removed

comment:8 Changed 9 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: assignedclosed

Fixed with [2106].

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