Opened 14 years ago

Closed 14 years ago

#5162 closed Bug (fixed)

Error in ajax sample

Reported by: Damian Owned by: Frederico Caldeira Knabben
Priority: Normal Milestone: CKEditor 3.2
Component: General Version: SVN (CKEditor) - OLD
Keywords: IBM Confirmed Cc: Joe Kavanagh

Description

To reproduce:

  1. Open Ajax sample on nightly build
  2. Add some content and destroy the editor
  3. Create the editor

the following error occurs in FF

this.document.getWindow().$ is undefined
[Break on this error] if(m)return;p.call(this);m=e.setTimeout(...urn s.type;var t=2,u=this.getNative();\r\n

in IE the data on the editor is not set.

Attachments (2)

5162.patch (1.7 KB) - added by Alfonso Martínez de Lizarrondo 14 years ago.
Proposed patch
5162_2.patch (1.3 KB) - added by Frederico Caldeira Knabben 14 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 14 years ago by Frederico Caldeira Knabben

Cc: Joe Kavanagh added; JOek removed
Keywords: Confirmed added

Issue introduced with [5015].

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

Keywords: Review? added
Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

I don't know if this should be fixed at the core (or how to fix it properly in an easy way).

The issue in Firefox seems to be that the call to setData the second time is done between the initialization of the instance and so it reaches a point in the code where the inner frame is loaded, but not finished with its initialization (or something weird along those lines, the document has null as its defaultView)

So in order to fix it, it's enough to do the setData call using the instanceReady event:

	editor = CKEDITOR.appendTo( 'editor' , {  on:{'instanceReady':function(e) { e.editor.setData( html )} } });

Besides that check, the first time that setData is called in this kind of environments, the Undo button shouldn't be available, so the call should be changed to

	editor.setData( html, null, false);

Which also fixes the problem (without using the event) because it no longer tries to create the initial snapshot.

So the correct code for the example should be

	editor = CKEDITOR.appendTo( 'editor' , {  on:{'instanceReady':function(e) { e.editor.setData( html, null, false )} } });

And that should always work.

The problem in IE is fixed by that code, but it was pointing to a little problem that can be fixed in the getSnapshot function: it returns false if the event doesn't return anything and the element has been appended, so it should be protected like the getData function.

Finally, maybe we could add a third parameter to CKEDITOR.appendTo allowing to specify the initial value for those instances, if that's OK it should be handled in a future ticket.

Another approach would be to internally delay any call to setData if the editor isn't initialized, but that seems more complex in my view (it will require a field to store the state of the editor and then adjust the behavior of setData according to that field with the instanceReady event)

I'm proposing a patch that just changes the sample and does the protection in getSnapshot, but that doesn't mean that there can be other better ideas.

Changed 14 years ago by Alfonso Martínez de Lizarrondo

Attachment: 5162.patch added

Proposed patch

comment:3 Changed 14 years ago by Frederico Caldeira Knabben

Owner: changed from Alfonso Martínez de Lizarrondo to Frederico Caldeira Knabben
Status: assignednew

You analysis is correct Alfonso, but the proposed solution could be a bit better, without making developers life harder.

I'll be proposing another patch for it.

comment:4 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

Changed 14 years ago by Frederico Caldeira Knabben

Attachment: 5162_2.patch added

comment:5 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Review? added; Review- removed
Status: newassigned

With the patch, the undo snapshot saving fails silently if there is no source of snapshot available, which is the basic problem here.

comment:6 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Review- added; Review? removed

The FF error is still there with this patch. I'm working on something else now.

comment:7 Changed 14 years ago by Frederico Caldeira Knabben

Keywords: Review- removed
Resolution: fixed
Status: assignedclosed

To not impact on the testing phase, I've fixed it directly with [5144]. Please feed free to reopen the ticket if the changes are not correct.

comment:8 Changed 14 years ago by Damian

Resolution: fixed
Status: closedreopened

This is still reproducible on IE.

comment:9 Changed 14 years ago by Frederico Caldeira Knabben

Resolution: fixed
Status: reopenedclosed

WFM after [5144]. In fact it breaks for me here (5143), but not here (5144).

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