Opened 10 years ago

Closed 9 years ago

#309 closed Bug (fixed)

Span with nonbreaking space added to button when adding paragraph break in IE

Reported by: miken Owned by: fredck
Priority: Normal Milestone: FCKeditor 2.6
Component: UI : Enter Key Version: FCKeditor 2.4.1
Keywords: Review+ Cc:

Description

Using IE, switch to source view, paste this code and then switch back to WYSIWYG view:

<p><button>1</button><button>2</button></p>

Placing the cursor between the two buttons and pressing enter to add a paragraph break adds a span with a non-breaking space inside the second button:

<p><button>1</button></p>
<p>&nbsp;</p>
<p><button><span>&nbsp;</span>2</button></p>

Attachments (2)

309.patch (891 bytes) - added by martinkou 9 years ago.
Proposed patch for fixing #309.
309_2.patch (1.3 KB) - added by fredck 9 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 9 years ago by fredck

  • Component changed from General to UI : Enter Key
  • Keywords Confirmed IE added; button paragraph p span nbsp removed
  • Milestone set to FCKeditor 2.6

Confirmed with IE6. With FF, we have a strange behavior which deserves a dedicated ticket.

In IE, we actually have an error when working with the SVN dev code. It seams that the remaning span is a marker for the selection.

comment:2 Changed 9 years ago by martinkou

  • Owner set to martinkou
  • Status changed from new to assigned

Changed 9 years ago by martinkou

Proposed patch for fixing #309.

comment:3 Changed 9 years ago by martinkou

  • Keywords Review? added; Confirmed IE removed

Took a while to figure out, but the fix for this is surprisingly simple.

comment:4 Changed 9 years ago by martinkou

The problem here comes from the FCKDomRange.MoveToElementEditStart() call in the enter key logic - it tries to move the caret into the button in the newly split paragraph. It is not possible to move the caret into a button in IE, however. Adding the BUTTON tag into FCKListsLib.EmptyElements() prevents MoveToElementEditStart() from moving the caret into the button.

comment:5 Changed 9 years ago by fredck

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

Your analysis is perfect Martin. The implementation has some problems though.

Adding "button" to EmptyElements is a dirty hack to fix this ticket. The comment right above the EmptyElements definition is quite precise: Elements marked as empty "Empty" in the XHTML DTD. The button element is not marked as empty, so it should not be added to that list. By doing so, we could easily have collateral effects. For example, it opens the possibility of having self-closing <button /> tags in the output, which is wrong.

Changed 9 years ago by fredck

comment:6 Changed 9 years ago by fredck

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

The proposed solution uses InlineNonEmptyElements instead of EmptyElements in the MoveToElementEditStart. It should satisfy the needs of MoveToElementEditStart.

comment:7 Changed 9 years ago by martinkou

  • Keywords Review+ added; Review? removed

comment:8 Changed 9 years ago by fredck

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

Fixed with [1404]. Click here for more info about our SVN system.

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