editors detach on AJAX form submission

Just confirming that this works like a charm with CKEditor.

Looks good to me too, except when submitting the form normally. The editors still get a full detach then, making things jump around.
I added a 'serialize' argument in the call to Drupal.wysiwygDetach() in Wysiwyg's event handler for reacting to form submissions to fix that.
Changed the comment over the detach() method in none.js to closer match the style in Drupal.detachBehaviors().
Also fixed jWysiwyg, which was still detaching at all times.

I hope there won't be any memory leaks from this, but I suppose we'll find out sooner or later.

Can you please confirm this is working as expected?

+++ b/editors/js/tinymce-2.js
@@ -67,7 +67,8 @@ Drupal.wysiwyg.editor.attach.tinymce = function(context, params, settings) {
+  trigger = trigger || 'unload';

+++ b/editors/js/tinymce-3.js
@@ -79,20 +79,25 @@ Drupal.wysiwyg.editor.attach.tinymce = function(context, params, settings) {
+  trigger = trigger || 'unload';

+++ b/editors/js/whizzywig-56.js
@@ -89,7 +89,8 @@ Drupal.wysiwyg.editor.attach.whizzywig = function(context, params, settings) {
+  trigger = trigger || 'unload';

+++ b/editors/js/whizzywig-60.js
@@ -38,7 +38,8 @@ Drupal.wysiwyg.editor.attach.whizzywig = function(context, params, settings) {
+  trigger = trigger || 'unload';

+++ b/editors/js/whizzywig.js
@@ -80,7 +80,8 @@ Drupal.wysiwyg.editor.attach.whizzywig = function(context, params, settings) {
+  trigger = trigger || 'unload';

Can't we enforce a default of 'unload' in the detach or wysiwygDetach method?

I don't like that part either, but it is possible to call all those methods directly from any code...
On the other hand, not having those checks there could send the signal that we don't wish those methods to be called directly.
I think I might actually like that better, as it goes along with what I had in mind for 3.x.

yep, that's in line with my thinking. Other scripts and external code should never call those editor-specific methods directly. If at all, then only our own, editor-specific integration code (within the same file) may call them.

Rerolled with the changes. The editor implementations now assume the trigger parameter is always present.

To speed things up, I'll commit this in a week unless something comes up.

EDIT: Just noticed I uploaded the wrong patch, it still has a some lines with trigger = trigger || 'unload' left there, will put the correct patch up later.

I had the same issue (with tinemce) and could fix it with #7.

Can someone update and applied the patch on #7 against the latest dev. version.
Thanks in advance.

I've committed the below patch to D7.
The -dev snapshots will be updated within 12 hours and this change will be in the next official D7 release.

I don't think we can fully backport this to D6 and/or D5 as we don't have the trigger argument or the detach behavior there, but if anyone has ideas, I'm interested. Marking this as 'needs backport' for now.

Yay, thanks!

For D6, at least our own form submit handler could pass the appropriate trigger. For any other calls, we'd default to 'unload' in wysiwygDetach(), so the previous behavior is retained.

That should fix things for regular submissions, but AJAXed submissions will still make the editors flicker and the page scroll uncontrollably. I think we'll still have other issues related to AJAX in D6 anyway, so maybe that's not a priority?

I have committed a backport of this for 6.x-2.x, without the detach() method in Drupal.behaviors.attachWysiwyg since there's no equivalent to it in D6.