Opened 7 years ago

Closed 7 years ago

#3887 closed Bug (fixed)

Create list command leaking into the rest of the document.

Reported by: martinkou Owned by: martinkou
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 martinkou 7 years ago.
3887_2.patch (2.0 KB) - added by martinkou 7 years ago.
3887_3.patch (2.0 KB) - added by martinkou 7 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 7 years ago by martinkou

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

comment:2 Changed 7 years ago by damo

  • Keywords IBM added

Changed 7 years ago by martinkou

comment:3 Changed 7 years ago by martinkou

  • Keywords Review? added

comment:4 Changed 7 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 7 years ago by martinkou

comment:5 Changed 7 years ago by martinkou

  • Keywords Review? added; Review- removed

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

comment:6 Changed 7 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 7 years ago by martinkou

comment:7 Changed 7 years ago by martinkou

  • Keywords Review? added; Review- removed

comment:8 Changed 7 years ago by garry.yao

  • Keywords Review+ added; Review? removed

comment:9 Changed 7 years ago by martinkou

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

Fixed with [3850].

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