setData doesn't clear properly the document listeners

[Alfonso Martínez de Lizarrondo]

The sample file is just the API.html with an extra listener to the 'contentDom' event that logs an entry in the console and then sets a click listener on the body:

contentDom : function(e) {

console.log('contentDom'); var doc = e.editor.document; doc.on('click', function() { console.log('click'); });

}

That setup works correctly in CKEditor 3, but in 4 it only works until setData is called. Then the contentDom event is logged, but the native listeners on the document remain (so they aren't attached again) although I think that the document itself is changed.

I mean: CKEditor thinks that the native listeners are already set for that element, but the element itself has changed and now no longer works until you switch from design to source and back forcing a full reset.

And this is why some people have been complaining that the onchange plugin doesn't work with CKEditor 4.

[Alfonso Martínez de Lizarrondo]

testcase

[Piotrek Koszuliński]

Hmm... I have a feeling that something has changed in Webkit recently, because previously we haven't seen need to use editable#attachListener for document events. In last two weeks I was fixing two issues with that - in both cases I had to change doc#on to editable#attachListener( doc ), because otherwise second and next listeners haven't been fired (unless iframe was completely replaced). And this issue was reproducible only manually - programatically fired events are triggering right listeners. Weird ;/

One of these tickets: #10330.

So if you agree I would close this issue. In any case - I'll try to find out a little bit more about this issue with document, because it is intriguing.

[Alfonso Martínez de Lizarrondo]

I'm testing with Firefox, so it's not just webkit.

[Piotrek Koszuliński]

Ok. Then I'll try to find if something changed in CKEditor since v3 regarding multiple listeners on document.

[Jakub Ś]

I think I have made similar "mistake" with AutoSave. As @Reinmar has explained this to me eventListeners should be assigned to editable and not document only. I didn't ask why just accepted it but if possible explanation would be welcome (probably because of design :) )

I'm attaching working sample.

I'm closing this issue as invalid. If you guys have however anything against, please comment or reopen ticket.

[Piotrek Koszuliński]

As I wrote before - I'm still going to check this, because in fact your case, Kuba, is another that proves that something has changed recently or in v4. But let's leave this ticket closed for now, I'll keep it in my TODOs.

[Piotrek Koszuliński]

It took us a while, but it looks like we found something: #11400.

[Piotrek Koszuliński]

And yes... now everything makes sense. As I mentioned in comment:1, using editable.attachListeners was helping for attaching a listener to document on second and following contentDom events. It did, because then those custom listeners are removed, so new one for the same event and function can be added. If removeAllListeners were correctly removing also custom listeners besides removing only native listeners, then there would be no need for using attachListeners, because after document.removeAllListeners done on editable.detach() everything would be reset to initial state.