Microsoft Asp.net 2.0 AJAX UpdatePanel Bug

[Anksunamon]

Hi,

I'm made a basic asp.net 2.0 Form with a FormView to edit a simple "text". -> My formview works with a FckEditor or a TextBox editor in it.

If I surround the formview with an UpdatePanel, Then It doesn't works anymore. The Text Inside the FckEditor is not recorded into the database. Neither when using insert nor update method. If I replace the FckEditor by a TextBox, then it works well again.

Does FckEditore don't support Microsoft Asp.net AJAX, or UpdatePanels, or I have to do something to the FckEditor to support it?

Thanks for helping me...

[fredck]

Do you think you can provide a test case for it. Something like a VS project with a page that shows the problem. It would help us a lot to reproduce it here, making it possible to fix it quicker.

[danparks]

Replying to fredck:

Do you think you can provide a test case for it. Something like a VS project with a page that shows the problem. It would help us a lot to reproduce it here, making it possible to fix it quicker.

I'm not the original poster, but I'm encountering the exact same problem.

An example would be pretty large. In short, I have a GridView and a FormView. The FormView has an FCKeditor in it. The editor gets its value from a database using Value='<%# Bind("ItemText") %>'. When I don't wrap the FormView in an UpdatePanel, it works (user changes in the FCKeditor get saved to the database). When I wrap the FormView in an UpdatePanel, the changes "don't stick."

[alfonsoml]

#1170 has been marked as dup

[justinmk]

Replying to danparks:

Replying to fredck:

Do you think you can provide a test case for it. Something like a VS project

When I don't wrap the FormView in an UpdatePanel, it works

Try setting the FCKeditor control as an <asp:AsyncPostbackTrigger> in the <Triggers> of the <UpdatePanel>. (I have not verified that this works.)

[fredck]

#1079 has been marked as DUP

[fredck]

#1672 has been marked as DUP. It also contains some useful links, like this.

[serializer]

See attached, I've inherited the original FCKeditor class and implemented IScriptControl. This ensures a client-side behaviour is attached which does nothing except to serve as a component placeholder which my WebForm_OnSubmit function can search for. This OnSubmit function is registered in an OnLoad override in the control, using ClientScript.RegisterOnSubmitStatement.

The Javascript OnSubmit routine should only be executed once per postback (but only if the FCK is *inside* an UpdatePanel...), and will iterate through all AJAX components to see if any are of type FCKeditorAjax, in which case it will fire the component's _onSubmit, which calls FCK's UpdateLinkedField method.

I've tested this with a single instance of FCK, but theoretically it should work with any number of them.

As I mentioned in #1672, this could be integrated with the original project but since it requires a reference to the AJAX libraries this could break existing applications.

It could however be included in an alternate project/solution file.

Usage:

- Reference assembly in web.config:

<add tagPrefix="fck" namespace="FredCK.FCKeditorV2.Ajax" assembly="FredCK.FCKeditorV2.Ajax"/>

- Use on page with <fck:FCKeditorAjax runat="server" />

Please let me know if this works for you!

[serializer]

Replaced attachment, a Resources file was in my original project which wasn't needed so I'd removed the file... unfortunately there was still a reference in AssemblyInfo.cs so the component wouldn't work. New version is fixed.

[serializer]

Please note: the attached fix works *but* if an Ajax refresh completely *removes* the editor from the DOM, a huge number of JS errors start getting generated. It appears some part of FCK is trying to communicate with the component, when it no longer exists. I'll be looking at fixing this fairly soon; my application requires this functionality.

[ClearCarbon]

serializer have you managed to fix the JS errors with your code? We require FCKEditor in our application with ajax support

[Anksunamon]

Under IE(7) fck inside an update panel WORKS. Under FIREFOX (2.0.0.12) fck inside an update panel DOESN'T WORKS.

PLEASE Correct this bug !

[serializer]

Anksunamon: Please try my patched version. It will produce JS errors but should work. (Unless this is a new problem in FF 2.0.0.12). Let me know if there are any unexpected problems.

ClearCarbon: No, I still haven't had time to look into the errors; however as stated above the editor will work anyway. The JS errors only start appearing if the FCK is removed from the page (during an AJAX refresh). The page will continue to work even with the errors. They are along the lines of '<some FCK component> does not exist'. I think some part of the FCK API is still trying to access the editor in the DOM after it has been removed. I'll check later on and post the full error message, in case anyone on the FCK team has an easy answer. (I need to know more about how to purge FCK from browser memory.)

Alternately, a fix could be made in the FCK core. Whichever line of code is producing the error, needs to first check whether the object it's looking for still exists. If it doesn't, it should stop trying! ;)

[fredck]

As stated in my first comment, we really need a simple test case page to work on. We need an aspx page, with the UpdatePanel, the FormView and FCKeditor, with precise indications on how to trigger the error.

Postponing it to the 2.6.1 to not block the 2.6 release.

[serializer]

Attached ASP.NET website contains 4 test pages:

1. FCKEditor.Net 2. FCKEditor.Net inside an UpdatePanel 3. FCKEditor.Net (AJAX fix) inside an UpdatePanel 4. FCKEditor.Net (AJAX fix) *without* UpdatePanel

We are most interested in #2 (demonstrates bug #234) and #3 (showing my fix as submitted, and the further Javascript errors).

Instructions: open one of the tests, change some text, click "Post". The text received by server will be displayed underneath the editor.

The tests have clarified a couple of points;

- To reproduce, just the UpdatePanel and FCKeditor are required; the FormView and GridView mentioned in the initial report are not related to the bug - The bug causes any editing done in the FCK to be lost; the initial value of FCKEditor.Value is always be posted back unchange - When using my patched version, everything is fine on the first postback. After the second postback, Javascript errors start appearing: FCK is not defined, fckeditorcode_gecko.js line 31 (from Firebug console). - I test in IE7, and discovered the fixed version worked absolutely fine with no errors. I then ran test #2 and discovered IT worked in IE7. So the bug could be just related to Firefox. Will test in more browsers!

Other notes: - Test #4 currently fails. This is because I shortsightedly threw an exception in the FCKeditorAjax component when no ScriptManager is available. Fix will follow shortly.

[serializer]

Further notes from stepping through script execution with Firebug...

I think the error begins with _AttachFormSubmitToAPI in fckeditorapi.js. It attaches an event listener to the parent form's 'submit' event. This seems to be getting fired after each AJAX postback, after the editor has been removed from the DOM By this time the editor must have been deleted from the DOM, and the FCK object doesn't exist, causing the errors. It all seems like MS and/or Firefox being a bit backward but that's what it looks like is happening...

I'm experimenting with adding a _DetachFormSubmitFromAPI function in fckeditorapi.js itself and finding some way to fire it on the AJAX component's dispose event (or similar).

-- Additionally I noted that in fckeditorapi.js you add an event listener on 'beforeunload' as well as 'unload'. However in this situation at least, the 'beforeunload' event doesn't get fired, just 'unload'; so the window.FCKUnloadFlag doesn't get set and the FCKeditorAPI_Cleanup() function does nothing. I'm not even sure what the purpose of FCKeditorAPI_ConfirmCleanup() is... But perhaps I could add a call to _DetachFormSubmitFromAPI into that Cleanup function? I think this would be a reasonable cleanup operation whatever the circumstance.

[justinmk]

serializer: I noticed in your comment on 01/04/08 21:58:44 you mention ClientScript.RegisterOnSubmitStatement. I haven't looked at your uploaded test code, so I apologize if you know this already, but thought I should mention that the Page.ClientScript javascript registration methods are not AJAX aware (whatever that means). the System.Web.Extensions.dll adds additional ScriptManager namespace under System.Web that must be used if a ScriptManager is detected on the page (e.g., in the case of an update panel).

so, in addition to the normal Page.ClientScript.RegisterOnSubmitStatement for a normal (non-AJAX) page, you must also check for a ScriptManager:

ScriptManager sm = ScriptManager.GetCurrent(Page);
if(null != sm)
{ //we are on an AJAX page
sm.RegisterOnSubmitStatement(...);
}

http://www.asp.net/AJAX/Documentation/Live/mref/M_System_Web_UI_ScriptManager_GetCurrent_1_9548b3ba.aspx

http://www.asp.net/AJAX/Documentation/Live/mref/O_T_System_Web_UI_ScriptManager_RegisterOnSubmitStatement.aspx

[serializer]

justinmk - that's how I already did it, and it works fine :). The problem seems to be FCK's *own* form.submit listener, which is being fired at a time when the editor no longer exists in the DOM. I think I need to detach the listener during the client behaviour's dipose method, but haven't quite figured out how :P.

[serializer]

The errors I am seeing would appear to be bug #1255.

This would mean #234 is itself resolved.

[serializer]

FCKeditor.Net AJAX test

[serializer]

FCKeditor.Net AJAX extension v1.0.2

[serializer]

Updated both attachments; removed the Exception from FCKeditorAjax - Test 4 now passes.

Tests up at: http://fcktest.pixelpunch.co.uk/

[serializer]

Test page moved to: http://fck.randomdev.co.uk

[serializer]

With regards resolving this bug (pending #1255), I suggest I create two additional project files within the FCKeditor.NET solution:

1. FredCK.FCKeditorV2.vs2005ajax.csproj

This will be compatible with .NET 2 / AJAX. This can't be built into the vanilla vs2005 project, since the AJAX dependencies (mainly on ScriptManager) may not always be available. I could either add the additional code directly into the FCKeditor component, and use #if directives to disable the code in other builds; or I could inlude the AJAX-compatible version as a second control, named FCKeditorAjax. I favour the first approach since then existing sites could be fixed just by swapping in the new .dll. The second approach would require additional work renaming any existing occurrences of the component.

2. FredCK.FCKeditorV2.vs2008.csproj

This will be a native VS2008 (targetting .NET 3.5) version of the project. Since 3.5 has AJAX bundled as standard, we can safely use only the AJAX-compatible component here. Since there are already vs2003 and vs2005 versions of the project, it makes sense to include a 2008 version in any case.

There is a third possible approach using reflection to determine at run-time whether the ScriptManager class is available and if so use it - then no additional vs2005ajax project would be required at all; however this idea is untested in these circumstances. This solution would also be less optimal due to additional processing costs involved in reflection, although this hit should be minor.

Any thoughts or preferences as to which approach should be taken?

[serializer]

I believe I've found a fix. By creating a DetachFormSubmit function in fckeditorapi.js, and adding a call to it in the Cleanup function, I thought I would have solved it. However there was an additional problem: the Cleanup routine wasn't getting called, because the "beforeunload" event wasn't getting fired. I'm not sure if this is due to the way ASP.NET handles destruction of iframes, or a general symptom of dynamically removing iframes in Firefox. Additionally, the ParentForm object didn't seem to be available at the time of unload, so it wasn't possible to detach the events then.

Either way, I attached an additional Cleanup function to the FCK object itself, which calls the normal onbeforeunload and onunload functions. This Cleanup function can be called manually before dynamically removing an editor, to ensure it is disposed correctly.

I can't yet confirm whether this fixes the general scenario of #1255, although it seems it should.

The onbeforeunload event handler merely sets a flag, which the onunload won't do anything without. This is apparently needed only to fix #183, and was added in rev. 546 (by martinkou). This is to prevent an issue when running inside a WebBrowser control. I wonder if there's another way to fix that issue, rather than requiring an extra call to a Cleanup function, to work around this side effect of it?

I'll upload a patch against trunk as soon as I've tested further against #1255.

[rafael_neri]

Hi,

When the FCKeditor this within a View in a MultiView an error of Javascript is generated. To correct this see FredCK is undefined.

Sorry for english.

protected override void OnLoad(EventArgs e)

{

base.OnLoad(e);

// Register the dummy function ScriptManager.RegisterOnSubmitStatement(this, typeof(FCKeditorAjax), "FCKeditorAjaxOnSubmit", "if(typeof(FredCK) != 'undefined'){FredCK.FCKeditorV2.Ajax.FCKeditorAjax.WebForm_OnSubmit();}");

}

[fredck]

There are also fix proposals for it at our Forums: http://www.fckeditor.net/forums/viewtopic.php?f=6&t=9826

[aeonflux]

Greetz. I have managed to fix my FCKEditor updates inside an UpdatePanel by following this article: http://jlcoady.net/archive/2007/03/30/fckeditor-work-inside-updatepanel

I still haven't tried the (more elegant) OnPreRender approach, as described in: http://forums.asp.net/t/1028530.aspx?PageIndex=4

My only problem now is the Javascript error in Firefox: "FCK is not defined" in fckeditorcode_gecko.js : line 31

[serializer]

aeonflux,

Please go back to the top of this page and read all the comments from the beginning.

As you will see, I have already developed a fix which is more elegant than either method you link to! It is packaged as a new version of the server control so really couldn't be easier.

As for "FCK is not defined", see ticket #1255. I've just realised I never uploaded my patch there, I'll get onto that ASAP after having a fresh look at it...

[cykophysh]

Just wanted to use the FCKAJAX component. However when I tried to compile tthe project it says

Warning 2 The referenced project '..\FCKeditor.Net\FredCK.FCKeditorV2.vs2008.csproj' does not exist. FredCK.FCKeditorV2.Ajax

Do I need to remove this reference and compile or am I missing a proejct?

[serializer]

Replying to cykophysh:

Ah, it needs a reference to the original FCKeditor.Net component. The supplied project just contains a class that extends the base class.

If you want you can remove that faulty reference and instead reference the V2 .dll, alternatively get the FCKeditor.Net source and reference it in a solution, as I had it (hence the broken project file!).

Unfortunately you'll still need a fix for #1255 in Firefox - I'll supply a fix for that tomorrow, as well as your issue fixed. It's really time I got onto resolving this finally...

[serializer]

Finally, I have the editor working perfectly inside an UpdatePanel!

I had to add a configuration option in fckconfig.js, which prevents the submit handler from being attached altogether. In FCKeditorAjax I test ScriptManager.SupportsPartialRendering and if true I set the option. Finally, I use ASP.NET's hooks to wire up my own submit event, as previously described.

I think this is the best solution since FCK's juggling of the submit event just seems to conflict with the AJAX framework and its own event handling. I think a "DisableSubmit" option is a good remedy for any situation like this. It's very easy to add our own submit event to call UpdateLinkedField, but it's very difficult to detach FCK's submit once it's been attached.

Unfortunately this won't fix #1255 which is itself caused by the dodgy event wiring I am now bypassing!

Can I get some feedback as to the best name for this setting? Something more descriptive like "PreventAttachSubmitEvent" or possibly instead have an "AttachSubmitHandler" option set to true by default?

I'll upload my current patch later today, I promise this time!

[fredck]

Hey serializer, I'm curious to see your patch. I was waiting to effectively start working on this ticket to give some feedback.

I think that a mixture of your suggestions is a good name for the configuration option:
FCKConfig.PreventSubmitHandler = false ;

I'll check your patch as soon as I put my hands on this ticket. Thanks!

[serializer]

Patch against FCKeditor trunk r2174 adding PreventSubmitHandler

[serializer]

AJAX control implementation utilising PreventSubmitHandler to fix #234

[serializer]

Patch and a new version of the control attached.

FCKeditor.Ajax.rar is now out of date and needs deleting! The new file (v0.9) should be used. I haven't updated the tests yet either. Thought I'd just get the files out rather than keep saying I'm going to and not doing ;)

Also apologies for confusing version numbers and bad file names ;) I seem to have regressed from 1.0.2 to 0.9.

Should finally note I haven't implemented the additional change suggested by rafael_neri. I was going to try to fix this by moving the RegisterOnSubmitStatement into the Render method, but haven't had time to test this. So in its current state, this control may cause errors if used inside a MultiView.

[fredck]

@serializer, the work you've done is pretty cool. The tests definitely helped a lot.

Before working on more aggressive strategies over the FCKeditor.Net code, I'm analyzing the ASP.Net AJAX code to have a better understanding of it. I found out that, after detecting that the MS AJAX code is present on the page, we could do something like this:

Sys.WebForms.PageRequestManager.getInstance()._onSubmitStatements.push( function()
	{
		FCKeditorAPI.GetInstance( 'FCK' ).UpdateLinkedField();
		return true;
	} );

Of course 'FCK' must be replaced with the instance name and so forth... but if you add the above script at the bottom of your Test2.aspx page, you will see that everything will just work. Then, we'll still have those JavaScript errors, but that is the easiest part of it.

Do you think the above is a trustable solution for it? I can propose a fix to be included into the FCKeditor core code that would use the above code instead of the normal submit handling in such cases.

Does that make sense?

[serializer]

I can think of a test case which could break that method:

You have a page with two update panels. One panel contains an fckeditor and a submit button. The other contains something else, for our test purposes just a submit button.

If the second panel gets updated, the fckeditor will remain as it is in the DOM - and none of the fckeditor code will runs when the postback is completed, so _onSubmitStatements.push will not get called. ASP.NET will throw away the old onSubmitStatements (I'm fairly sure they'll get trashed and re-created with each postback). So now there is no submit handler to call UpdateLinkedField the *next* time a postback happens.

I'll put this test case together and add it to the others, in case I'm not describing it clearly... I'm not definitely sure it'll fail but I suspect it will. I'll also update the tests to a patched FCK so we can see that working...

[fredck]

I've attached a patch that uses the pure JavaScript approach to solve this problem. It seems to work well in all different cases.

It was tested with .Net 2.0 only... it should work with the 3.5 also... test results are welcome.

@serializer, I'm not sure I understood your last test case idea, but I've tested this patch on some different situations and it worked well. I'll be waiting for your ok before putting this patch under review.

Thanks for all your collaboration on this.

[craig_m]

Replying to fredck:

I've attached a patch that uses the pure JavaScript approach to solve this problem. It seems to work well in all different cases. It was tested with .Net 2.0 only... it should work with the 3.5 also... test results are welcome.

Hi there,

I am experiencing this problem in my application (.Net 2) in Firefox so I tried applying your patch, but it didn't seem to fix it. I then tried applying it to serializer's "Test 2" in case it was something else wierd about my page, but again it didn't work.

All I did was copy lines 150 to 186 in the patch file to the _AttachFormSubmitToAPI() function in fckeditorapi.js. I tried putting an alert in the function just to check it was being called, but it didn't get displayed so i'm not sure if it is being called at all! Is there something else that needs to be done to get it working?

Thanks

[serializer]

Craig,

You need to add ?fcksource=true onto your URL. This will mean your modified files in _source will be used, instead of the packaged fckeditorcode_gecko.js and fckeditorcode_ie.js, which are distributed with the editor.

Once you've finished testing, you can then use "fckpackager" to compile the files yourself, then ?fcksource=true is no longer needed.

The query string is checked by the FCKeditor.Net assembly, and if it finds fcksource=true then it uses "fckeditor.original.html" instead of "fckeditor.html". This version references the original _source files instead of the packaged ones.

[craig_m]

Replying to serializer:

You need to add ?fcksource=true onto your URL. This will mean your modified files in _source will be used, instead of the packaged fckeditorcode_gecko.js and fckeditorcode_ie.js, which are distributed with the editor. Once you've finished testing, you can then use "fckpackager" to compile the files yourself, then ?fcksource=true is no longer needed.

Thanks serializer, that worked a treat!

Great solution guys, well done.

[fredck]

Replying to craig_m:

Thanks serializer, that worked a treat!

Which one worked for you? The 234.patch or serializer's solution?

[craig_m]

Replying to fredck:

Which one worked for you? The 234.patch or serializer's solution?

It was 234.patch that I was referring to above. I tried serializer's solution first and it didn't seem to work, but I guess this was for the same reason that 234.patch didn't work for me initially.

234.patch seems more elegant than serializer's solution, with less set up involved. However, if this was part of an official release I guess this wouldn't be an issue, so it's really down to which solution gives the correct behaviour under all conditions.

[serializer]

FredCK's patch is better in that it doesn't require an extra .js include - which my AJAX solution does. However in terms of architecture, it's bringing ASP.NET-specific code into the fckeditor core, breaking the usual separation of the server-specific implementions as separate components from the core editor.

Another concern is that the _onSubmitStatements object is an internal feature of PageRequestManager which *could* be subject to change in future releases, since it's not publicly specified or documented. (Unlikely I admit, but a potential gotcha...)

Another problem would be if a different Javascript framework decided to name something Sys.WebForms.PageRequestManager... again very unlikely but would cause unexpected problems!

A further problem I'm not sure about, is what would happen if the fckeditorapi code ran *before* the PageRequestManager had loaded? Personally I don't know enough about the Javascript pipeline in different browsers to know how possible this is. Basically, are scripts *guaranteed* to be executed in the order that they are referenced in the page, or are they just executed as soon as they're downloaded?

The other test case I outlined (comment 35) could be easily worked around by using the endRequest event of the PRM, there re-attaching our onsubmit event in case it has been deleted during the async operation. This might not even be necessary, I should have some time tomorrow to test this.

Here's the official PageRequestManager reference: ASP.NET Documentation

It's pretty annoying that there's no publicly-exposed way to attach a submit event. "beginRequest" and "initializeRequest" only seem to be triggered for async postback, but we need something that will be triggered for ordinary (synchronous) postbacks as well. So the only way officially endorsed is via the server-side RegisterOnSubmitStatement method, pretty strange for a client-side framework!

The following note is at the end of the PRM reference:

"This class contains private members that support the client-script infrastructure and are not intended to be used directly from your code. Names of private members begin with an underscore ( _ )."

So _onSubmitStatements is considered dangerous territory... but on the other hand, it works!

[fredck]

Part 1 of 2 : Targeted to the FCKeditor trunk

[fredck]

Part 2 of 2 : Targeted to the FCKeditor.Net trunk

[fredck]

Replying to serializer:

FredCK's patch is better in that it doesn't require an extra .js include - which my AJAX solution does. However in terms of architecture, it's bringing ASP.NET-specific code into the fckeditor core, breaking the usual separation of the server-specific implementions as separate components from the core editor.

I was aware of that and actually checked the effective impact of it in the core code... ~370 Bytes. So, that would not be a big issue, considering the importance of this compatibility fix.

Another concern is that the _onSubmitStatements object is an internal feature of PageRequestManager which *could* be subject to change in future releases, since it's not publicly specified or documented. (Unlikely I admit, but a potential gotcha...)

I was pretty unhappy with it too. I've actually minimized the usage of "private" stuff to avoid possible incompatibilities. I've also checked if anything got changed in this sense since the .Net 2.0 and the 3.5 and everything we use here looks the same. But, this thing still makes me worried too. Only MS knows what could happen with .Net 4.0 or whatever. It could work for a temporary solution though. We always have the chance to fix it in the future.

Another problem would be if a different Javascript framework decided to name something Sys.WebForms.PageRequestManager... again very unlikely but would cause unexpected problems!

Unlikely to happen, but I've added it into a "try" block just in case.

A further problem I'm not sure about, is what would happen if the fckeditorapi code ran *before* the PageRequestManager had loaded? Personally I don't know enough about the Javascript pipeline in different browsers to know how possible this is. Basically, are scripts *guaranteed* to be executed in the order that they are referenced in the page, or are they just executed as soon as they're downloaded?

This is not an issue because the API will get called only when the editor is loaded, far later the page loading completion.

---

So, 234.patch is still a possible solution. I've avoided other things to not introduce unwanted dependencies to the FCKeditor.Net assembly.

But, I still wanted to give it a try by using reflection. So, here we have the 234_2_FCKeditor.patch and 234_2_FCKeditor_Net.patch patch files for it. It also introduces the PreventSubmitHandler setting.

What do you guys think about this one now? What should be the way to go?

[fredck]

@serializer, @craig_m : Any comment on this? I have to commit this thing to the upcoming release, and I'm still unsure which option to pick for it. I like my latest proposal, but I need some feedback to be sure it is ok in that way.

[craig_m]

Replying to fredck:

@serializer, @craig_m : Any comment on this? I have to commit this thing to the upcoming release, and I'm still unsure which option to pick for it. I like my latest proposal, but I need some feedback to be sure it is ok in that way.

Hi Fred,

I have just tested your new solution (234_2_FCKeditor.patch and 234_2_FCKeditor_Net.patch) in my app and it seemed to work fine. Although this would require users to update to the latest versions of both components (*), I think it is a cleaner solution and I believe it resolves the concerns raised by serializer regarding your previous solution.

*While I was applying your patch I noticed that the new solution seemed to work without the js patch. I tested this again by commenting out the changed lines and clearing the cache on the browser to ensure I had the latest files. If this is the case it may be that the main editor patch does not need to be applied to the next release, and therefore users will only need to update the .Net component to fix this problem?

Cheers,

Craig

[fredck]

@craig_m : Thanks for the tests! I'm happy to hear it worked for you.

The changes in the core are needed to introduce the "PreventSubmitHandler" setting, avoiding the "FCK is not defined" error to happen when reloading the posted editor in IE. This error happened consistently to me.

Thanks again! I'll just wait for serializer's comments to move on this thing.

[serializer]

@FredCK: Sorry for causing delay here.

Basically, I still prefer my original method - because it fits in neatly with the AJAX interface, rather than having to use Reflection which is less maintainable and less obvious what is going on.

However yours is more straightforward in that it does not require a separate build to support backwards compatibility, and it does not produce the additional .js include that I did.

Really, both approaches are doing exactly the same thing; I just moved my client script into a separate resource rather than directly writing code in the RegisterOnSubmitStatement call. To this end your version outputs slightly more code on each page than mine did, at the tradeoff of one less include, which was pretty small anyway.

Really the pros/cons of the two approaches aren't big enough in either case to be worth worrying about. I definitely prefer this method to your first patch, so I think we have a consensus that 234_2_FCKeditor.patch and 234_2_FCKeditor_Net.patch are the way to go.

Just one minor suggestion: instead of overriding the OnLoad method, how about moving that code into OnPreRender? The main reason is if the control has Visible=false, then it won't exist on the page, and so the submit statement won't be needed. OnPreRender only gets called if the control is actually getting rendered.

[fredck]

Thanks for the comments serializer. Let's go ahead with 234_2 for now and check the feedback.

I'm asking for review, mainly for the core changes.

[fredck]

Sorry... serializer's suggestion for using OnPreRender makes sense... I'll work on a new patch for it.

[fredck]

Part 1 of 2 : Targeted to the FCKeditor trunk

[fredck]

Part 2 of 2 : Targeted to the FCKeditor.Net trunk

[fredck]

Ok, I've attached a new set of patches including serializer's proposition of using the OnPreRender method instead of OnLoad. Good one.

[serializer]

Great, will be nice to see it finally wrapped up!

[fredck]

Fixed with [2281] and [2282].

Special thanks to serializer. This fix would never come up without his great contributions.

Thanks also to craig_m and all others who have helped us on testing, giving their invaluable feedback.

[kipusoep]

Replying to fredck:

Fixed with [2281] and [2282]. Special thanks to serializer. This fix would never come up without his great contributions. Thanks also to craig_m and all others who have helped us on testing, giving their invaluable feedback.

I am using your new version at the moment and I had to change something.

I'm afraid you forgot that people can use their own implementation of the ScriptManager, or the AjaxControlToolkit Scriptmanager, which has a BaseType of System.Web.UI.ScriptManager.

So wouldn't it be better to check for the BaseType, or if that isn't enough the Type and BaseType?

[serializer]

I don't think even BaseType would be enough for all possible scenarios. What if I subclassed the AjaxControlToolkit Scriptmanager itself to create a further implementation? Then, the BaseType would be the AjaxControlToolkit implementation, not System.Web.UI.ScriptManager.

Instead, use the Type.IsSubClassOf method http://msdn.microsoft.com/en-us/library/system.type.issubclassof.aspx to check the type. Then any number of derivations will be supported.

[kipusoep]

Replying to serializer:

I don't think even BaseType would be enough for all possible scenarios. What if I subclassed the AjaxControlToolkit Scriptmanager itself to create a further implementation? Then, the BaseType would be the AjaxControlToolkit implementation, not System.Web.UI.ScriptManager. Instead, use the Type.IsSubClassOf method http://msdn.microsoft.com/en-us/library/system.type.issubclassof.aspx to check the type. Then any number of derivations will be supported.

Yes, you are right. My post was just meant to show a problem with the current version. I knew this wasn't the best solution, but I didn't have the time to search for 'the better' solution :)

[kipusoep]

But is this going to be changed in the new version...?

[serializer]

I had a look at doing this, and it's hard to call the IsSubClass method purely using reflection, since we need a reference to the base type itself.

I'm a bit wary of doing anything too complex in reflection since it can break Medium Trust scenarios (which I'm currently working in).

A fix for this definitely should be included in a future release, and thanks for bringing the problem to light.

[jskripsky]

The code in PreRender() is much slower than it needs to be. There are two areas that can be improved:

Finding the page's ScriptManager

Use the static method ScriptManager.GetCurrent(Page p) instead of recursively going through the page's control tree. Using ScriptManager.GetCurrent works for both "plain" System.Web.Extensions and AjaxControlToolkit (using ToolkitScriptManager).

Use of Reflection

Do the Type.GetMethod()/Type.GetProperty() calls only once. Do it at class initialization time and save the resulting MethodInfo/PropertyInfo for later use.

I'll attach a patch implementing both ideas.

[jskripsky]

Patch implementing the above mentioned optimizations

[kipusoep]

Replying to jskripsky:

The code in PreRender() is much slower than it needs to be. There are two areas that can be improved: == Finding the page's ScriptManager == Use the static method ScriptManager.GetCurrent(Page p) instead of recursively going through the page's control tree. Using ScriptManager.GetCurrent works for both "plain" System.Web.Extensions and AjaxControlToolkit (using ToolkitScriptManager). == Use of Reflection == Do the Type.GetMethod()/Type.GetProperty() calls only once. Do it at class initialization time and save the resulting MethodInfo/PropertyInfo for later use. I'll attach a patch implementing both ideas.

FCK has to be backwards compatible with .NET 2.0 Correct me if I'm wrong, but GetCurrent() isn't backwards compatible. I think that's the reason for choosing Reflection too...

[jskripsky]

Replying to kipusoep:

FCK has to be backwards compatible with .NET 2.0 Correct me if I'm wrong, but GetCurrent() isn't backwards compatible. I think that's the reason for choosing Reflection too...

My code uses GetCurrent() only via reflection. On .NET 2.0

Type.GetType( "System.Web.UI.ScriptManager, System.Web.Extensions" )

will return null. As a result "getCurrentScriptManagerMethod" will be null. Hence we'll exit early in PreRender:

if ( !_IsCompatible || getCurrentScriptManagerMethod == null ) 
     return;

No harm done.

On the other hand, when "System.Web.UI.ScriptManager, System.Web.Extensions" is known, all the MethodInfo/PropertyInfo will be != null and invoked.

[serializer]

Replying to jskripsky:

I was just going to point that out, but you were too quick ;)

Jskripsky thanks for this solution, it addresses all my previous concerns, in particular the slow traversal up the control hierarchy as you pointed out. (Not so much of an issue in a small test page; but the project I'm working on has a vast and complex control hierarchy and I really was worried about the speed factor!) I'd started looking at fixing this but hadn't yet had time so you've saved me a fair bit of work :) .

This should also address the issues of subclassing ScriptManager as raised by kipusoep, since GetCurrent should quite happily return a reference to whatever ScriptManager (descended or otherwise) is managing the page.

I'm reopening the ticket since I consider this patch provides critical improvements to the package - pending some tests to ensure it all works of course ;)

[serializer]

Replying to jskripsky:

{{{ Type.GetType( "System.Web.UI.ScriptManager, System.Web.Extensions" ) }}}

This line doesn't work (in my scenario at least).

Testing locally this returns null (with System.Web.Extensions loaded).

If I change it to:

Type.GetType( "System.Web.UI.ScriptManager, System.Web.Extensions",true )

Then I can see that a FileNotFoundException is being thrown.

I think it might be because my GAC contains two different versions of System.Web.Extensions (Frameworks 2.0 and 3.5). This situation is also quite likely on shared hosting.

To get it working I had to change the line to:

Type scriptManagerType = Type.GetType( 
"System.Web.UI.ScriptManager, System.Web.Extensions, Version=3.5.0.0, 
Culture=neutral, PublicKeyToken=31BF3856AD364E35" ) ;

Obviously this references a specific version and so wouldn't work with Framework 2.0. I've seen a method a Rick Strahl's blog (http://www.west-wind.com/WebLog/posts/10246.aspx) which called AppDomain.CurrentDomain.GetAssemblies() then looped through to find System.Web.Extensions. But this doesn't seem very optimal either, and I'm pretty sure the GetAssemblies() call will fail in medium trust.

So, I don't quite know what to do here. Perhaps we could just have two calls to Type.GetType, and check for both possible versions of System.Web.Extensions. As and when newer versions of AJAX are released, this will need updating.

[jskripsky]

Replying to serializer:

Replying to jskripsky:

{{{ Type.GetType( "System.Web.UI.ScriptManager, System.Web.Extensions" ) }}}

This line doesn't work (in my scenario at least).

You're right, Type.GetType() needs a full assembly name including version and pubkey token to load from the GAC. I was using a heavily customized version of Mono for the testing...

Obviously this references a specific version and so wouldn't work with Framework 2.0. I've seen a method a Rick Strahl's blog (http://www.west-wind.com/WebLog/posts/10246.aspx) which called AppDomain.CurrentDomain.GetAssemblies() then looped through to find System.Web.Extensions. But this doesn't seem very optimal either, and I'm pretty sure the GetAssemblies() call will fail in medium trust. So, I don't quite know what to do here. Perhaps we could just have two calls to Type.GetType, and check for both possible versions of System.Web.Extensions. As and when newer versions of AJAX are released, this will need updating.

I did some research on possible options. Strahl's approach indeed seems to be our best bet. According to http://www.west-wind.com/weblog/posts/9601.aspx (search for "trust"), it should work with medium trust as well.

[fredck]

You guys have messed up this ticket a bit, by reopening it. We have already worked on a fix for this, committed and released it. If something else is to be done about it, a new dedicated ticket should be opened. It's a pity that I've checked it just now (shame on me).

Do you think you are able to summarize the latest discussions in a new ticket now? Thanks!

[kipusoep]

@ serializer

I think you're better in creating tickets than me ;-) I'm new here, these were my first posts and you can describe the problem (and maybe a possible solution) better.