Opened 12 years ago

Closed 12 years ago

#105 closed Bug (fixed)

oEditor.GetParentForm() is not a function

Reported by: anonymous Owned by: Alfonso Martínez de Lizarrondo
Priority: Normal Milestone: FCKeditor 2.4.1
Component: General Version: FCKeditor 2.4
Keywords: Cc:

Description

Receive this error when trying to submit a form. This error started showing up in 2.4

The error console pointed to line 36 in ./js/fckeditorcode_gecko.js

if (oEditor.GetParentForm() == this )oEditor.UpdateLinkedField();

putting a try/catch block around this and alerting the value of oEditor.GetParentForm, while in the catch block, always alerts null. Placing some sort of define check inside the conditional seems to fix the problem. My patch:

if (oEditor.GetParentForm && oEditor.GetParentForm() == this )oEditor.UpdateLinkedField();

Change History (7)

comment:1 in reply to:  description Changed 12 years ago by josh.ritter@…

One more thing. I confirmed this to be a problem in firefox2.0, and ie7.

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

It works fine here.

Could you please provide detailed steps to reproduce?

comment:3 Changed 12 years ago by josh.ritter@…

In my attempt to get in a testing enviroment I could point you at I nailed down the cause. We use our own home grown application framework (solstice.eplt.washington.edu). Anyway, we just upgraded our version of json. The new version does a lot of object.prototype work. It was actually mucking around with your Instances array, so that this code alerts twice, even though I only have 1 fckeditor on my page:

for ( var name in FCKeditorAPI.Instances ){ alert(name); ... }

If you want to see this problem, I've setup a sample that is removed from our framework, and just contains the fckeditor code, plus an included json.js file. (http://staff.washington.edu/jdr99/test.html). Just hit the submit link.

Turns out the newest version of json is causing us problems elsewhere also, so we are just going to use an earlier version.

Thanks for your attention to this matter.

comment:4 Changed 12 years ago by Alfonso Martínez de Lizarrondo

Owner: set to Alfonso Martínez de Lizarrondo
Status: newassigned

I've checked in [165] a patch for this problem.

I'm not sure if this should go to the main trunk. It fixes the problem and it shouldn't have any drawback. The root of the problem is the use of Object.prototype by the JSON library (http://therealcrisp.xs4all.nl/blog/2006/12/04/tojsonstring-and-objectprototype/) but even if that library is changed, other people might want to do similar things and it can break again.

So this is a fix for a problem generated by external code, but it should be better to be protected instead of ignoring it and have the editor broken when the fix is available.

comment:5 in reply to:  4 Changed 12 years ago by Frederico Caldeira Knabben

Replying to alfonsoml:

I've checked in [165] a patch for this problem.

I think you wanted to say [163] Alfonso :)

I'm not sure if this should go to the main trunk. It fixes the problem and it shouldn't have any drawback.

So this is a fix for a problem generated by external code, but it should be better to be protected instead of ignoring it and have the editor broken when the fix is available.

I also think it is beter to protect ourselves from outside. So a check is ok, but maybe instead of looking outside the current object for the check (the Object.prototype I mean), maybe it is better to check the object itself. Something like:

    'if ( oEditor.GetParentForm && oEditor.GetParentForm() == this )' + 
        'oEditor.UpdateLinkedField() ;' + 

In this way we are ready for any possible way of object "infection" (people are really creative out there!).

And maybe also a small comment explaining why we are doing it would be good too :) I'm seeing myself saying "Strange thing... We don't need it" in a future code cleanup (v 3.0).

Ok to "trunk" it ;)

comment:6 Changed 12 years ago by Frederico Caldeira Knabben

Milestone: FCKeditor 2.4FCKeditor 2.4.1

comment:7 Changed 12 years ago by Alfonso Martínez de Lizarrondo

Resolution: fixed
Status: assignedclosed

I've commited to trunk in [173] the new code.

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