Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#823 closed defect (fixed)

form.submit() fix

Reported by: gogo Owned by: gogo
Priority: normal Milestone: Version 1.0
Component: Xinha Core Version: trunk
Severity: normal Keywords:
Cc:

Description

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

Change History (19)

comment:1 Changed 11 years ago by guest

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

comment:2 Changed 11 years ago by 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

comment:3 Changed 11 years ago by 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.

comment:4 Changed 11 years ago by 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.

comment:5 Changed 11 years ago by 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.

comment:6 Changed 11 years ago by wymsy

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

comment:7 Changed 11 years ago by 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.

comment:8 Changed 11 years ago by 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.

comment:9 Changed 11 years ago by 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???

comment:10 Changed 11 years ago by wymsy

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

comment:11 Changed 10 years ago by wymsy

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

Committed in changeset:599.

comment:12 Changed 10 years ago by ray

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:13 Changed 10 years ago by 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.

comment:14 Changed 10 years ago by 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.

comment:15 Changed 10 years ago by 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){}
    }
  }

comment:16 Changed 10 years ago by 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.

comment:17 Changed 10 years ago by wymsy

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

comment:18 Changed 10 years ago by ray

Yeah, this seems simple and reasonalbe

comment:19 Changed 10 years ago by ray

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

Note: See TracTickets for help on using tickets.