Opened 8 years ago

Closed 8 years ago

#3887 closed Bug (fixed)

Create list command leaking into the rest of the document.

Reported by: Martin Kou Owned by: Martin Kou
Priority: Normal Milestone: CKEditor 3.0
Component: Core : Lists Version: SVN (CKEditor) - OLD
Keywords: IBM Review+ Cc:

Description

The symptom of this bug seems similar to #3780, but so far I've found it to be caused by the domiterator and thus it is a actually very different beast.

To reproduce:

  1. Open replacebyclass.html in Firefox.
  2. Create a default table.
  3. Press Shift-Enter in any of the table cells.
  4. Press up to move the caret back to the beginning of the table cell.
  5. You'll see the list command has leaked into all table cells that comes after the currently selected one, and then also to the rest of the document.

This bug was discovered while working on #3782.

Attachments (3)

3887.patch (2.1 KB) - added by Martin Kou 8 years ago.
3887_2.patch (2.0 KB) - added by Martin Kou 8 years ago.
3887_3.patch (2.0 KB) - added by Martin Kou 8 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 8 years ago by Martin Kou

This bug seems to be a regression from [3609].

comment:2 Changed 8 years ago by Damian

Keywords: IBM added

Changed 8 years ago by Martin Kou

Attachment: 3887.patch added

comment:3 Changed 8 years ago by Martin Kou

Keywords: Review? added

comment:4 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

The patch works, but for safety, I would suggest to not only limit to empty text node but any empty spaces:

this._.lastNode.type == CKEDITOR.NODE_TEXT && this._.lastNode.getLength() == 0

Even further, how do you think of this more generic:
source:CKEditor/trunk/_source/plugins/domiterator/plugin.js#L53- L61

var walker = new CKEDITOR.dom.walker( range ),
bookmarkTextGuard = CKEDITOR.dom.walker.bookmark( true, true ),
// Reject any empty spaces.
emptySpacesGuard = CKEDITOR.dom.walker.emptySpaces( true ),
ignoreBookmarkTextEvaluator = function( node )
{
return bookmarkTextGuard( node ) || emptySpacesGuard( node ) 
};

Changed 8 years ago by Martin Kou

Attachment: 3887_2.patch added

comment:5 Changed 8 years ago by Martin Kou

Keywords: Review? added; Review- removed

The evaluator approach doesn't work because the walker never touched the empty text node anyway.

comment:6 Changed 8 years ago by Garry Yao

Keywords: Review- added; Review? removed

Can you please use:

!CKEDITOR.tool.trim( this._.lastNode.getText() )

Instead of

/^[ \t\r\n]*$/.test( this._.lastNode.getText( )

Changed 8 years ago by Martin Kou

Attachment: 3887_3.patch added

comment:7 Changed 8 years ago by Martin Kou

Keywords: Review? added; Review- removed

comment:8 Changed 8 years ago by Garry Yao

Keywords: Review+ added; Review? removed

comment:9 Changed 8 years ago by Martin Kou

Resolution: fixed
Status: newclosed

Fixed with [3850].

Click here for more info about our SVN system.

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