Opened 6 years ago

Closed 4 years ago

#10571 closed Bug (fixed)

filler char should be removed prior to undoImage

Reported by: Paul Huck Owned by:
Priority: Normal Milestone:
Component: Core : Selection Version: 4.1.2
Keywords: Cc:

Description

The filler char is not removed prior taking an undo image. As a result, the undo image contains the character, but a restoration of that image means that the character will not be removed. This is because the filler char is found by looking at the custom data on the editable, however, the image is restored by setting the html of the editable, and consequently the node stored in the custom data is no longer in the editable (but a copy is).

Steps to reproduce (using chrome):

  1. Move the selection programatically via range.select() which results in a filler char
  2. Add some more text to the filler char node
  3. create undo image (editor.fire('saveSnapshot'))
  4. restore the image

Results: the filler char remains in the document and cannot be removed.

I believe this can be fixed by adding the following line in selection.js (line 420) editor.on('beforeUndoImage', function (evt) { removeFillingChar(editable); });

Change History (4)

comment:2 Changed 6 years ago by Piotrek Koszuliński

Status: newconfirmed

Thanks for your patch :). In fact, I've been already thinking whether this approach will work and I even reported #10332.

We need to investigate this very carefully, so the review will take a while, because currently we're very busy with other things.

comment:3 Changed 4 years ago by Piotrek Koszuliński

I closed the PR, because it turned out to be incorrect - https://github.com/ckeditor/ckeditor-dev/pull/48#issuecomment-56950275

comment:4 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: confirmedclosed

I'm not sure if I checked this case precisely before confirming this issue, but I can't confirm it now.

There's code in the selection.js that does the job - https://github.com/ckeditor/ckeditor-dev/blob/b2f1ee102c80a41469c59f2d744e675d44ee0f25/core/selection.js#L857-L897 and from my tests I can see that it does it well. I couldn't find a case in which ZWS might leak into the snapshot.

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