Opened 3 years ago

Closed 3 years ago

#11909 closed New Feature (fixed)

Parameter to prevent creating snapshots on editor#setData

Reported by: Jakub Ś Owned by: Marek Lewandowski
Priority: Normal Milestone: CKEditor 4.4.2
Component: General Version: 4.4.0
Keywords: Cc:

Description (last modified by Marek Lewandowski)

Editor setData method now generates undo snapshot (Please see: #5217).

Right after the release we got two questions from users that were benefiting from editor.setData('') not generating snapShots.
Their request is that setData should have extra parameter that blocks snapShot execution what in result can block change event and leave dirty flag set to false.

Things like that can be easily worked around with below code, thus I would like to ask if there is anyone for whom below workarounds don't work and parameter is the only solution?

var editor = CKEDITOR.replace( 'editor1', {});
editor.on('instanceReady', function(){
	console.log('Before setData:'+editor.checkDirty());
	editor.setData('',function(){
		console.log('Inside setData before reset:'+editor.checkDirty());
		editor.resetDirty();	
		console.log('Inside setData after reset:'+editor.checkDirty());
	});

});
var editor = CKEDITOR.replace( 'editor1', {});	
editor.on('instanceReady', function(){
	editor.setData('');				
});
editor.on('afterSetData', function(){
	console.log('After setData before reset:'+editor.checkDirty());
	editor.resetDirty();
	console.log('After setData after reset:'+editor.checkDirty());
},null,null,100);

Change History (12)

comment:1 Changed 3 years ago by Jakub Ś

Status: newconfirmed

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

Description: modified (diff)
Summary: Parameter to block firing snapShots in setDataParameter to prevent creating snapshots on editor#setData

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

cc

comment:4 Changed 3 years ago by Wiktor Walc

Just to clarify the use cases reported by customers that I was missing:

Case 1

Workflow:

  1. Load a page with the editor.
  1. Type some text into the editor: “A”
  1. We have a reset button that calls setData(“”) on the editor. Click the reset button, or do something that triggers a call so setData(“”).
  1. Type some new text into the editor: “B”
  1. Hit the Undo button. (All the text clears out)
  1. Notice that the Undo button is still active suggesting that the undo button can be clicked again.
  1. Hit the Undo button again. The old text “A” is loaded back into the editor

Case 2

After a recent update from 4.3 to 4.4 we noticed a problem that we were only able to fix by overriding the setData method in the editor class.

We have a form with a bunch of fields, including ckeditor field(s). After saving the form, we clear the ckeditor field(s) and mark the form as ‘not dirty’.

In a simplified scenario, with 4.3, we were able to do this:

// this is actually a loop that sets values to all fields on the form 
// synchronously, unless one of the fields is a ckeditor
me.editor.setData(''); 

form.isDirty = false;

With 4.4 now, setData fires the saveSnapshot event which triggers the change event which will cause the isDirty flag to be set to true.

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

The case 1. can be easily solved by using resetUndo and resetDirty in setData's callback. However, the case 2. is more troublesome and shows clearly that the best solution will be if we introduce such option for setData. Of course there are workarounds like - cancelling specific saveSnapshot events, but that's not a nice thing.

The only problem I have is the parameters order in setData. Currently we have: data, callback, internal. The last parameter is mostly for editor's internal usage, but it's documented in the API so it may be uncool to change it. So the new parameter would have to be added at the end and this starts to look bad.

Last edited 3 years ago by Piotrek Koszuliński (previous) (diff)

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

Milestone: CKEditor 4.4.2

Fred's idea is to group all parameters in an options object. Of course backward compatibility for callback and internal attributes will be kept. We already did that in a couple of other methods.

We'll handle this in a 4.4.2 because it is a small, backward compatible change.

Proposal:

  • options
    • callback
    • internal
    • noSnapshots

comment:7 Changed 3 years ago by Marek Lewandowski

Description: modified (diff)

comment:8 Changed 3 years ago by Marek Lewandowski

Description: modified (diff)

comment:9 Changed 3 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedassigned

comment:10 Changed 3 years ago by Marek Lewandowski

Status: assignedreview

Pushed to t/11909 at dev.

I've reused existing getData() test which was also testing setData(). I've added also saveSnapshot event tracking.

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

Status: reviewreview_passed

I force pushed branch:t/11909 after many small improvements.

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

Resolution: fixed
Status: review_passedclosed

Merged to master in git:7ffe649.

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