form.submit() fix

[gogo]

From noxi in forum ( http://xinha.gogo.co.nz/punbb/viewtopic.php?id=758 )

if (!this._textArea.form.$XINHA_submit){
        this._textArea.form.$XINHA_submit = this._textArea.form.submit;
        this._textArea.form.submit = 
            function(){
                for(var i=0;i < this.elements.length; i++){
                    var element = this.elements[i];
                    if (element.type != 'textarea') continue;

                    for (var a=0; a  < __htmlareas.length;a++){
                        var editor = __htmlareas[a];
                        if (editor._textArea == element){
                            element.value = editor.outwardHtml(editor.getHTML());
                        }
                    }//for - each editor
                }//for - each element
                this.$XINHA_submit();
            }//function
    }//if

---

[guest]

Nice, but where would you put this piece of code?

[mokhet]

in the generate() function, just after setting the onSubmit and onReset handlers. I've apply it in the branch /branches/mokhet, changeset:548

Here is the patch to apply to trunk revision, can someone test it before commiting to trunk ?

Index: htmlarea.js
===================================================================
--- htmlarea.js	(revision 548)
+++ htmlarea.js	(working copy)
@@ -1618,6 +1618,30 @@
         return true;
       }
     );
+
+    if ( !textarea.form.$XINHA_submit )
+    {
+      textarea.form.$XINHA_submit = textarea.form.submit;
+      textarea.form.submit = function()
+      {
+        for ( var i = this.elements.length; i--; )
+        {
+          var element = this.elements[i];
+          if ( element.type == 'textarea' )
+          {
+            for ( var a = __htmlareas.length; a--; )
+            {
+              var editor = __htmlareas[a];
+              if ( editor && editor._textArea == element)
+              {
+                element.value = editor.outwardHtml(editor.getHTML());
+              }
+            }
+          }
+          this.$XINHA_submit();
+        }
+      };
+    }
   }
 
   // add a handler for the "back/forward" case -- on body.unload we save

[wymsy]

Mokhet, I tried your patch in my cms, but it didn't work. I didn't try to debug it, but I did try the version posted in the ticket, and that one does work for me. My tests were in Firefox 1.5.0.6 on Mac OS X.

[mokhet]

Well, what can i say, if you look at the original and the patch they are doing exactly the same things. It is working ok with Fx1.5.0.6 win2K.

Only differences i see are

• textarea.form instead of this._textArea.form but they are the same
• the boucle for is reversed (i-- vs i++) but that's also the same

So i dont see what can be wrong with one and not the other. If you have the "original" working, can you submit the diff patch to understand what's wrong with mine ? thanks.

However, with IE i have issue with this patch too. This afternoon i have disabled it in the branch i'm working on. I'll commit updates tomorrow.

[wymsy]

There is a third difference. You changed

if (element.type != 'textarea') continue;

to

if ( element.type == 'textarea' ) {
}

Strangely, while these two constructs appear logically equivalent, they don't give the same result. I haven't quite figured out the details, but I think it has to do with items in the elements array that are not actually form elements (such as fieldsets, for example) and therefore don't have a type attribute. Anyway, changing your version back to the "continue" construct makes it work again in FF. I still haven't tried it in IE - my testing server is down right now.

[wymsy]

It appears to be working OK in IE also, (with the "continue" construct).

[mokhet]

Good catch I can't believe i didnt see it. It's ok for IE too ? Great. I'll try that in a few. Yeah sorry, i promissed my updates yesterday and it's still not commited. Damn real life, once it le me get 2 hours free, promiss, i'll commit.

[mokhet]

Can you submit a patch, or at least attach your working version please ? Because even with the "continue" construct i keep getting IE errors.

[wymsy]

This code works in IE on my cms:

    if ( !textarea.form.$XINHA_submit )
    {
      textarea.form.$XINHA_submit = textarea.form.submit;
      textarea.form.submit = function()
      {
        for ( var i = this.elements.length; i--; )
        {
          var element = this.elements[i];
          if ( element.type != 'textarea' ) continue;
          for ( var a = __htmlareas.length; a--; )
          {
            var editor = __htmlareas[a];
            if ( editor && editor._textArea == element)
            {
              element.value = editor.outwardHtml(editor.getHTML());
            }
          }
        }
        this.$XINHA_submit();
      };
    }

If it doesn't work for you, then maybe it depends on what is in the elements array, or some environment issue like that???

[wymsy]

Is it okay to commit this (maybe change $XINHA_submit to xinha_submit)?

[wymsy]

Committed in changeset:599.

[ray]

There seems to be a problem with that patch, see #869

[wymsy]

At least part of the problem is that a name conflict arises if the page has a submit button with id="submit". In that case, textarea.form.submit refers to the button instead of the submit() method this patch is trying to modify. Since it is very common to name/id submit buttons as 'submit', this patch won't work unless there is another way to reference the submit() method (I haven't come up with one yet).

There is probably more, but I haven't been able to reproduce the error in #869 exactly yet.

[wymsy]

On further reflection, I think the situation is not so bad after all. What this patch is doing is intercepting the submit() method and adding a line of code to copy the iframe into the textarea before submitting the form. The submit() method is only called if javascript is used to submit the form, not if a submit button is used. Therefore, this patch is not needed when there is a submit button, therefore we can simply put the patch in a try/catch block. I have tried this on the full_example (installed on my server) and it works.

I guess you could come up with situations where the form could be submitted either via a button or a script, or some other form element was named 'submit', but these would be unusual, to say the least, and in any event would not break xinha. They would just require an onsubmit() as we have now.

[wymsy]

Here is the modified code. Anyone care to try it before I commit it?

    // save the iframe content to the textarea before a form.submit()
    // note: catch error in IE if any form element has id='submit'
    if ( !textarea.form.xinha_submit )
    {
      try
      {
        textarea.form.xinha_submit = textarea.form.submit;
        textarea.form.submit = function()
        {
          for ( var i = this.elements.length; i--; )
          {
            var element = this.elements[i];
            if ( !element.type || element.type != 'textarea' ) continue;
            for ( var a = __htmlareas.length; a--; )
            {
              var editor = __htmlareas[a];
              if ( editor && editor._textArea == element)
              {
                element.value = editor.outwardHtml(editor.getHTML());
              }
            }
          }
          this.xinha_submit();
        };
      } catch(ex){}
    }
  }

[wymsy]

Actually, I am not going to commit the above code, because I found a simpler and better way to do it. Better because it works in the case where an editor has been made (with HTMLArea.makeEditors(...)), but not yet generated (with HTMLArea.startEditors(xinha_editors)). (For an example of why you might want to do this, see http://xinha.org/punbb/viewtopic.php?id=483.)

    // attach onsubmit handler to form.submit()
    // note: catch error in IE if any form element has id='submit'
    if ( !textarea.form.xinha_submit )
    {
    	  try {
		textarea.form.xinha_submit = textarea.form.submit;
		textarea.form.submit = function() {
			this.onsubmit();
			this.xinha_submit();
		};
	  } catch(ex) {}
    };

I have committed this in changeset:607.

[ray]

Yeah, this seems simple and reasonalbe

[ray]

This change mysteriously disappeared between [607] and [601]. Re-applied in [730]