Ticket #2519 (closed Bug: fixed)

Opened 6 years ago

Last modified 6 years ago

Firefox 3 form submit via javascript with multiple editor instances

Reported by: ccarey Owned by: martinkou
Priority: High Milestone: FCKeditor 2.6.4
Component: General Version: FCKeditor 2.6.3
Keywords: Confirmed Firefox Review+ Cc: ccarey@…

Description

Summary:

If you create a form which contains multiple instances of the FCK Editor, Firefox3 will only send updates to one of the instances if you submit the form via javascript (form.submit()). Submitting the same form via a "native" submit button (input type="submit") will send the form as expected.

See sample form attached (modified sample file as detailed below)



Steps to replicate:

  1. Open the sample09.html bundled with the editor fckeditor/_samples/html/sample09.html



  1. Edit the sample code to add a second button at the bottom which submits the form via javascript
<input type="submit" value="Submit (button)" />
<input type="button" value="Submit (javascript)" onclick="document.testform.submit()"/>
  1. Run the sample in firefox 3.0.1



  1. Edit the values in both editor regions to "XX" and submit the form using "native" submit button. Notice that the receiving page shows the correct data ("XX").



  1. run the sample again and change the field values to "YY" and submit the form using the javascript button. Notice that the receiving page shows only one of the regions updated (i.e. the first region shows "YY" and the other shows the default value)



Notes

  • tested in version 2.6.3 and nightly build (2008-09-02)
  • behaviour not present in IE7 or FF2.x
  • Inspecting the hidden "value" field at the point where the form is submitted (FF3) shows the value only being updated for one instance of the editor at a time.



It's a bit of an issue if you're using external validation (like qForms) or if the page design requires that the activation buttons for the form are outside the form boundaries.

Attachments

sample09.html (3.6 KB) - added by ccarey 6 years ago.
"Sample 9" from bundled HTML examples modified to include javascript submit button
2519.patch (2.0 KB) - added by martinkou 6 years ago.

Change History

Changed 6 years ago by ccarey

"Sample 9" from bundled HTML examples modified to include javascript submit button

comment:1 Changed 6 years ago by ccarey

  • Cc ccarey@… added

comment:2 Changed 6 years ago by arczi

  • Keywords Pending WorksForMe added; Firefox 3, multiple instance, form submit removed
  • Summary changed from Firefox 3 form submit via javascript with multiple editor instances - to Firefox 3 form submit via javascript with multiple editor instances

I was unable to reproduce this bug. Could you write something more?

comment:3 Changed 6 years ago by fredck

  • Keywords Confirmed Firefox added; Pending WorksForMe removed

I was able to reproduce the problem with FF3. It is quite inconsistent though... sometimes only the first editor is updated... other times only the second... and sometimes both.

Works well with IE.

comment:4 Changed 6 years ago by ccarey

It's sometimes difficult to replicate the exact circumstances - clearing the cache (or not) between attempts appears to have no bearing on the result. I haven't yet found a method of making it consistently fail.

comment:5 Changed 6 years ago by ccarey

Here's something that may be of help - we found today that running the uncompressed source (fckeditor.original.html) is much more stable under Firefox3.

The problem boils down to the init routine running twice.

I hacked some code into the compressed version (fckeditorcode_gecko.js) to show me what editors were in the collection at each stage during startup. Line numbers are meaningless here since it's all compressed so I'll just list the location

This code was added right after FCKeditorAPI.Instances[FCK.Name]=FCK;

/* show me what you have stored */
console.log("------------------------------------------------");
window.console.log('++ add instance : ' + FCKeditorAPI.Instances[FCK.Name]);
console.log("++ editors in collection ++");
	var inst = FCKeditorAPI.__Instances;
	for(var o in inst) console.log(o);
};

... and this was added right after window.FCKeditorAPI = {

/* tell me when you run */
window.FCKeditorAPI = {logOutput:console.log("init - instances cleared"),

The output is this:

init - instances cleared
------------------------------------------------
++ add instance : middle_column_heading
++ editors in collection ++
middle_column_heading
------------------------------------------------
++ add instance : middle_column_content
++ editors in collection ++
middle_column_heading
middle_column_content
init - instances cleared
------------------------------------------------
++ add instance : left_column_header
++ editors in collection ++
left_column_header

Note that the string "init - instances cleared" appears twice. I've added three instances, but only one ends up in the collection since the collection itself is cleared sometime after the second editor is created (on this run -- sometimes the init runs sooner, resulting in only the first instance being killed).

There's a copy of the hacked file here: http://staging.fi.net.au/fckeditor/hacked_fckeditorcode_gecko.js

Making the same edits to the uncompressed fckeditorapi.js results in this output:

 ....loading....
Firebug's log limit has been reached. %S entries not shown.		Preferences	 
init - instances cleared
------------------------------------------------
++ add instance : left_column_header
++ editors in collection ++
left_column_header
------------------------------------------------
++ add instance : middle_column_heading
++ editors in collection ++
left_column_header
middle_column_heading
------------------------------------------------
++ add instance : middle_column_content
++ editors in collection ++
left_column_header
middle_column_heading
middle_column_content

Note that the init routine only reports running once at the very start (as you'd expect)

comment:6 follow-up: ↓ 7 Changed 6 years ago by ccarey

quick update - the thing still fails every so often with the uncompressed source (same fault as above) but less frequently

comment:7 in reply to: ↑ 6 Changed 6 years ago by george_a

FF 3.0.0 Cache Off; FCK 2.6.3.

I have 4 editors on page. 3 of them in <div style="display:hidden"> And after loading page im have in FCKeditorAPI.Instances only one editor reference;

Im change code to: var A = window.parent; if(!(FCKeditorAPI = A.FCKeditorAPI)){

++ alert('Create new');

....

And on loading page im see 4 alerts 'Create new';

It is wrong

Im change code again:

var A = window.parent; ++ alert(window.parent); if(!(FCKeditorAPI = A.FCKeditorAPI)){

++ alert('Create new');

And im see 4 alerts with 'Object' and only one alerts with 'Create new'.

It`s well.

May be parent.window ever not exists then executing this expression.

Sorry for my English...

comment:8 Changed 6 years ago by fredck

  • Milestone set to FCKeditor 2.6.4

I've been informed that this bug has been introduced with the 2.6.3, and that things used to work well with the 2.6.2.

The hidden fields update logic is handled by the FCKeditorAPI code. The only change made on it between versions 2.6.2 and 2.6.3 was [2188]. Can anyone confirm that this bug get fixed by simply reverting those changes?

comment:9 Changed 6 years ago by martinkou

Reverting [2188] fixed the bug for me.

comment:10 Changed 6 years ago by martinkou

  • Owner set to martinkou
  • Status changed from new to assigned

comment:11 Changed 6 years ago by wwalc

kon_ at #fckeditor irc channel also confirmed that reverting [2188] fixes this bug.

One more thing about this bug (quoting _kon):

"the cause lies here according to matthias miller: https://bugzilla.mozilla.org/show_bug.cgi?id=383682"

comment:12 Changed 6 years ago by martinkou

  • Priority changed from Normal to High

I've found another approach to fix the bug that would need reverting [2188] (which makes #1907 reappear).

What really caused the problem was that the previous code expected a certain sequence of execution:

  1. Instance 1 defines FCKeditorAPI, and thus FCKeditorAPI.Instances.
  2. Instance 2 redefines FCKeditorAPI, and overwrites FCKeditorAPI.Instances.
  3. Instance 1 and 2 add themselves to FCKeditorAPI.Instances.

Because Instance 1 and Instance 2 are executed in different windows, there's really no guarantee that step 3 will always happen after step 1 and 2 - race condition. This is what happened in this ticket - Instance 1 added itself to FCKeditorAPI.Instances before Instance 2 overwrote it and cleared it. Thus, after [2188] in sample09.html, either FCKeditorAPI.GetInstance('FCKeditor_Default') or FCKeditorAPI.GetInstance('FCKeditor_Basic') would give you undefined.

The real fix to this is to change FCKeditorAPI's code to handle the race condition, instead of restoring the hack to make the race condition less likely to happen.

comment:13 Changed 6 years ago by martinkou

  • Keywords Review? added

comment:14 Changed 6 years ago by martinkou

Typo: "that wouldn't need reverting..."

Changed 6 years ago by martinkou

comment:15 Changed 6 years ago by alfonsoml

  • Keywords Review+ added; Review? removed

I haven't been able to reproduce the bug, but the patch seems a safe solution.

comment:16 Changed 6 years ago by martinkou

  • Status changed from assigned to closed
  • Resolution set to fixed

Fixed with [2562].

Click here for more info about our SVN system.

comment:17 Changed 6 years ago by alfonsoml

#2566 has been marked as dup

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