OnChange event

[giammin]

Why ckeditor doesn't have a onchange event. It would be very useful to alert the editor of unsaved changes mainly in inline edit mode

[fredck]

We've taked extensively about it on #900, but we're now decided to have it inside the editor as a better promoted feature.

[fredck]

My current idea, to help on easy to implement and less performance impact, is making this featured based on the undo plugin. In fact, the "undo" button reacts in the exactly same way one would expect this event to happen.

We may either have it as a undo plugin feature or a dedicated plugin, depending on the performance impact of it.

[Wim Leers]

Sounds sensible :)

[giammin]

maybe a config parameter: fireonchangeevent=true

[Wim Leers]

Replying to giammin:

maybe a config parameter: fireonchangeevent=true

That would be fine, at least from a Drupal perspective.

[giammin]

I don't get the dupal keyword...

this ticket is not drupal related. I don't even use drupal.

[Wim Leers]

Replying to giammin:

I don't get the dupal keyword...

this ticket is not drupal related. I don't even use drupal.

This is definitely not Drupal-specific. The reason it is tagged with "Drupal" is simply because this is one of the things that CKEditor doesn't already have and that Drupal needs. Drupal 8 is going to ship with CKEditor 4, and that is why it is tagged: to indicate which tickets are related to getting CKEditor to ship with Drupal.

See ​http://buytaert.net/from-aloha-to-ckeditor.

[giammin]

Replying to Wim Leers:

This is definitely not Drupal-specific. The reason it is tagged with "Drupal" is simply because this is one of the things that CKEditor doesn't already have and that Drupal needs. Drupal 8 is going to ship with CKEditor 4, and that is why it is tagged: to indicate which tickets are related to getting CKEditor to ship with Drupal.

See ​http://buytaert.net/from-aloha-to-ckeditor.

Ok,thanks now i get it!

[wwalc]

Just to not forget: the onchange plugin has already been used for some time.

[inghamc]

Looking forward to this...

[dahjelle]

Looking forward to this as well! Since the onchange plugin isn't "officially" supported in v4, this is holding us back from upgrading just yet. Keep up the good work!

[jonnott]

+1 for this .. what's the eta for v4.2?

[Wim Leers]

Drupal's current emulation of this feature is the following:

    var editor = CKEDITOR.dom.element.get(element).getEditor();
    if (editor) {
      var changed = function () {
        callback(editor.getData());
      };
      // @todo Make this more elegant once http://dev.ckeditor.com/ticket/9794
      // is fixed.
      editor.on('key', changed);
      editor.on('paste', changed);
      editor.on('afterCommandExec', changed);
    }

Sadly, editor.getData() does not yet contain the changes made by the latest keypress!

E.g.: the value is fooba. You type an 'r' at the end of this, now the visible value is foobar. But editor.getData() will still return fooba!

The temporary solution I found was to change the changed callback to wrap the call to editor.getData() in a window.setTimeout… not very nice, but it works for now:

      var changed = function () {
        window.setTimeout(function () {
          callback(editor.getdata());
        }, 0);
      };

---

Drupal.org issue: ​http://drupal.org/node/1995730

[Reinmar]

It turns out that there is an input event which is fired when contenteditable element is changed.

​https://developer.mozilla.org/en-US/docs/Web/Reference/Events/input?redirectlocale=en-US&redirectslug=DOM%2FMozilla_event_reference%2Finput

Have anyone tested it? It seems to be implemented in every browser we support (except Opera, but it switches to Blink). I wonder how broken it is :D

Edit: It works pretty well. It is triggered by D&D and typing.

[Wim Leers]

Very interesting! Did you test it in every supported browser already; does it work "pretty well" in all of them? :)

[Reinmar]

Just a quick check on FF and Chrome and it works fine - it is fired on every key and drag&drop (but programatic DOM changes aren't triggering it).

In fact, we are planning (but it's hard to tell now if it is going to work) to implement onchange event using undo manager. So this event may not be required for onchange specifically, but to improve undo manager which e.g. has problems with D&D (#7422).

[fredck]

Our current idea is firing the "dirty" event, instead of "change". It would be named after the ​checkDirty method.

This means that we would not have the event fired constantly, for every single change in the editor, but instead just once, to indicated that the editor has changes. This in fact seems to be the most common requirement for it, so the hosting app UI can react to the dirty state.

This would be a much lighter event in this way, in fact.

Would this be enough? Opinions are welcome.

[Reinmar]

1. First thing that comes to my mind is that developers may have troubles with finding this event and understanding it right. The "change" event is well known from DOM, when the "dirty" flag is rarely used. So even googling may not help, because "ckeditor onchange" may not return anything related to this event before it will be popularized on StackOverflow, our forum etc. And this can take a lot time, because for very long there was no official solution, so old answers will be misleading.

So - one thing is consistency, but second - intuitiveness. I'd say that better to have latter one in this case, because we're making it mostly for other developers.

2. When this event will be fired? Like we've been talking - together with undo?

3. If "dirty" was fired there won't be another "dirty" event unless I call editor.resetDirty(). Do I understand correctly?

4. Regardless of whether 3) is true - consider such case:
   1. I listen on "dirty".
   2. When it is fired I getData() and try to save them via XHR (and call resetDirty() if have to).
   3. Now... when will be next "dirty" fired? If there's no forced gap between them, then we can have a situation when next one is fired nearly immediately what can cause issues with concurrency.

Of course, developers should be able to implement data saving correctly, but let's face it - most of them won't know that such situation can happen and great majority of others won't know how to solve this.

Thus, I think that there should be a configurable timeout between subsequent "dirty" events. It can be easily implemented using ​https://github.com/cksource/ckeditor-dev/blob/t/9764/core/tools.js#L1004 It's not a complete solution for concurrency, but at least something making this situation less probable.

[Wim Leers]

Good points by @Reinmar.

For Drupal's purposes, an isDirty event would be sufficient. But as @Reinmar says, it may be counterintuitive. And in fact, his 4th point brings us very close to an onChange event, with the caveat that it is not fired every time, but only X times per second.

I think that's the best compromise: let there be a changeInterval setting or something like that, by which the user can say "only fire the onChange event at most every 250 ms". CKEditor can then even default to a very conservative value, e.g. 2000 ms.

Note: this does not mean the first event should also be delayed by 2000 ms: the first one should fire immediately, but after that it should only fire every 2000 ms.

[Reinmar]

PS. My main concern is that this event should be useful for developers without deep knowledge about editor, problems caused by asynchronous execution etc. I know that there's a place for 3rd party plugins to solve all these issues, but we shouldn't simplify our event too much, because the whole addition will be useless in 90% cases.

[j.swiderski]

"dirty" event. but instead just once, to indicated that the editor has changes

How will this work? Will event inform that something was changed and we can do with it whatever we want?
What about undo manager - what will happen when user reverts changes to moment before event was fired? Perhaps this should be flag plus event?

[jonnott]

From my developer-perspective, what I need is way of knowing if/when *anything* has changed in the editor's output since the editor was initialised. Currently I'm only able to check for key events, i.e. via:

CKEDITOR.replace('editordiv').on('key',function(){ this.updateElement(); });

I'm using FormSavior (​https://github.com/ianand2/Form-Savior), and this is the only way I've been able to make it detect that something's changed.. Hence why a 'change' event would be perfect.

[Reinmar]

Replying to Wim Leers:

I think that's the best compromise: let there be a changeInterval setting or something like that, by which the user can say "only fire the onChange event at most every 250 ms". CKEditor can then even default to a very conservative value, e.g. 2000 ms.

Note: this does not mean the first event should also be delayed by 2000 ms: the first one should fire immediately, but after that it should only fire every 2000 ms.

This is how eventsBuffer works - first "event" triggers output immediately, but a next one not earlier than given timeout after the previous one.

On the other hand - there's also no point in sending data to server after first key was pressed. Usually it's better to buffer them. So if we want to make something useful for saving data, then we should start from a nice algorithm for saving data which buffers changes in reasonable way and hide part of it under the CKEditor's API.

If we'll start designing this from "we should have an event" perspective, then we can end up with something that can't be easily used.

[wwalc]

So correct me if I'm wrong, the correct flow to write an "autosave" feature would be to:

• listen for the onChange event
• when onChange is triggered, call resetDirty() immediately after getting the data from the editor

This way it would be impossible to lose the last few characters typed by the user in case of leaving the page by the user straight after the last onChange was called. The isDirty flag will let us know properly that such user has still unsave changes.

That looks fine to me. The only issue that I see here, eventually, is that dirty flag / resetDirty() is global, so there might be a problem if another 3rd party plugin is resetting the dirty flag, because it is using it for "something".

[fredck]

I think you guys got it already. The idea about "dirty" is not bringing a weird name for the "change" event. It is in fact a different kind of event.

It is supposed to be fired whenever the "dirty" status of the editor changes to "true". In other words, calling ​checkDirty after that will give you "true".

Then, i would not get fired any more, unless a new change happens following a ​resetDirty call.

I understand that there is also a potential requirement for the "change" event, that's why I'm calling for opinions.

Btw, when it comes to timeouts... this is out of the scope of the editor. Firing an event, which is not used by the editor code, doesn't bring performance issues to the editor. It's the holding app the responsible to deal with concurrency or whatever other issue related to event frequency, based on it own requirements and parameters. Well, at least for the first implementation, so we can KISS.

[wwalc]

The KISS approach with dirty looks reasonable to me. Actually the more I read about this, the more I see that having timeouts directly in CKEditor makes very little sense. After all if someone writes an autosave or something, he should simply do it smartly. We cannot predict all nonoptimal usages of any event and try to protect against it.

Still, reducing the onchage event to a dirty event and a global flag seems to have one drawback: suppose one wants to have

• a word counter that counts the number of words in the editing area (people seem to use such things: http://ckeditor.com/addon/wordcount)
• and have an autosave feature / code that checks if there are any unsaved changes

both use cases seems suitable to listen to the onchange event and do something when it is triggered, each having of course an internal interval to not make time/resources consuming actions too often.

[giammin]

I think that the onchange plugin works quite good.

I use this code:

config.minimumChangeMilliseconds = 500;	

CKEDITORCUSTOM.setEditorSavedState = function (editor, isUnsaved) {
	editor.unsaved = isUnsaved;
	var element = document.getElementById(editor.name);
	if (isUnsaved) {
		if (element.className.indexOf(' ckeditorUnsaved') === -1) {
			element.className = element.className + ' ckeditorUnsaved';
		}
	} else {
		element.className = element.className.replace(' ckeditorUnsaved', '');
		editor.resetDirty();
	}
};
	
CKEDITOR.on('instanceReady', function (ev) {
	ev.editor.on('change', function () {
		if (!ev.editor.unsaved && ev.editor.checkDirty()) {
			CKEDITORCUSTOM.setEditorSavedState(ev.editor, true);			
		}
	});
});

the only problem is that the event is fired when a modal(link, image,etc) is opened and closed without any modification.

[Reinmar]

"KISS" is good if its result still solve some problem. I'm not saying that "dirty" event does not make sense - I don't know this. That's why I said that we should have use cases first (live if possible) and we'll verify if "dirty" at least partially help in solving them. Because there's no point in introducing useless API.

Already mentioned use cases:

• saving data,
• word counter,
• ?

Does "dirty" really help in implementing them? If not - in what it helps?

The only issue that I see here, eventually, is that dirty flag / resetDirty() is global, so there might be a problem if another 3rd party plugin is resetting the dirty flag, because it is using it for "something".

I agree. This is in fact... a very important drawback. We'll make checkDirt() useless. I even remember issues reported for conflicting resetDirty() calls.

[Reinmar]

If we'll remove the requirement to call resetDirty(), then it doesn't make sense any more to call this event "dirty". So I'd change its name to "change" and voila! We have an event which is fired after every single change in editor.

Used together with eventsBuffer in its listener it can be a good base for autosave data and word counter.

[fredck]

The use case for "dirty" is Drupal, were the event is used to simply enable the save button, nothing else. It's similar to something like once('change').

I'm still unsure about how simpler having "dirty" can be when compared to "change". If it'll be the same effort, then ofc "change" makes more sense. We gonna probably figure this out when starting the implementation.

[m.guzek]

ok, I am going to start working on it. The summary of requirements before starting coding will appear here asap. Any further propositions / conclusions / suggestions very welcome.

[m.guzek]

After some investigations, we decide to have "change event" not the "dirty event" and put requirements in the following way:

• change event functionality is the part of undo plugin,
• change event is triggered AFTER the change,
• attaching handlers to event is done by "on('change')" method of editor object,
• each change in code MUST trigger change event (1 char change as well),
• executing the command (Bold, Italic, etc. ) MUST trigger change event,
• undo, redo MUST trigger change event

Comments still welcome.

[jonnott]

The above specification sounds just right to me.

[m.guzek]

Working draft (kinda proof of concept) available for preview ​https://github.com/cksource/ckeditor-dev/commit/6265ce7af73c27a571d50f37b9bb9128fc52a2c1 .