Opened 3 years ago

Closed 3 years ago

#11909 closed New Feature (fixed)

Parameter to prevent creating snapshots on editor#setData

Reported by: j.swiderski Owned by: m.lewandowski
Priority: Normal Milestone: CKEditor 4.4.2
Component: General Version: 4.4.0
Keywords: Cc:

Description (last modified by m.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 j.swiderski

  • Status changed from new to confirmed

comment:2 Changed 3 years ago by Reinmar

  • Description modified (diff)
  • Summary changed from Parameter to block firing snapShots in setData to Parameter to prevent creating snapshots on editor#setData

comment:3 Changed 3 years ago by Reinmar

cc

comment:4 Changed 3 years ago by wwalc

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 Reinmar

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 Reinmar (previous) (diff)

comment:6 Changed 3 years ago by Reinmar

  • Milestone set to 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 m.lewandowski

  • Description modified (diff)

comment:8 Changed 3 years ago by m.lewandowski

  • Description modified (diff)

comment:9 Changed 3 years ago by m.lewandowski

  • Owner set to m.lewandowski
  • Status changed from confirmed to assigned

comment:10 Changed 3 years ago by m.lewandowski

  • Status changed from assigned to review

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 Reinmar

  • Status changed from review to review_passed

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

comment:12 Changed 3 years ago by Reinmar

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

Merged to master in git:7ffe649.

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