Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#10365 closed Bug (invalid)

setData doesn't clear properly the document listeners

Reported by: Alfonso Martínez de Lizarrondo Owned by:
Priority: Normal Milestone:
Component: Core : Editable Version: 4.0
Keywords: Cc:

Description

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.

Attachments (2)

api.html (7.0 KB) - added by Alfonso Martínez de Lizarrondo 11 years ago.
testcase
api7.html (7.1 KB) - added by Jakub Ś 11 years ago.

Download all attachments as: .zip

Change History (9)

Changed 11 years ago by Alfonso Martínez de Lizarrondo

Attachment: api.html added

testcase

comment:1 Changed 11 years ago by 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.

comment:2 Changed 11 years ago by Alfonso Martínez de Lizarrondo

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

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

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

comment:4 Changed 11 years ago by Jakub Ś

Resolution: invalid
Status: newclosed

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.

Changed 11 years ago by Jakub Ś

Attachment: api7.html added

comment:5 Changed 11 years ago by 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.

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

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

comment:7 Changed 10 years ago by 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.

Version 0, edited 10 years ago by Piotrek Koszuliński (next)
Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy