Opened 9 years ago

Closed 8 years ago

#735 closed defect (fixed)

IE eating javascript (again?)

Reported by: benDOTsimkinsATintsoftDOTch Owned by: gogo
Priority: normal Milestone:
Component: Xinha Core Version: trunk
Severity: major Keywords: script tag ie sethtml textmode wysiwyg
Cc:

Description

When changing from text to wysiwyg mode, IE appears to be eating my javascript, in spite of the javascript/freezescript replacement proposed by mharrisonline.
Firefox works fine, haven't tried anything else.

[sob]

Just paste this

<script type="text/javascript">
  alert("does it work? no it doesn't");
</script>

in text mode, in IE, and switch to wysiwyg and back, and it's not there anymore.

It is NOT a problem with getHTML() - if you save directly from textmode, it works fine. The problem occurs in setHTML() (line 5196 and following), you can demonstrate this by replacing it with the following code:

// completely change the HTML inside
HTMLArea.prototype.setHTML = function(html)
{
  if ( !this.config.fullPage )
  {
    alert(html);
    this._doc.body.innerHTML = html;
    alert(this._doc.body.innerHTML);
  }
  else
  {
    this.setFullHTML(html);
  }
  this._textArea.value = html;
};

Did this used to work? Has someone broken something?

Change History (14)

comment:1 Changed 9 years ago by mharrisonline

The fix that I proposed takes content that is opened into Xinha and nullifies the pre-existing JavaScript? by changing the language to the fictional FreezeScript?, which the browser then won't execute. It then converts the language back to JavaScript? upon save.

However, if you insert JavaScript? into Xinha manually after it is already open, you would either need to insert it with FreezeScript? already the scripting language, or else modify Xinha to convert it for you. You could do that by adding the regular expression to the end of the HTMLencode function, or if you are using the GetHTML plugin, to it's JS file.

I'm working on changing my proposed code for that patch.

comment:2 Changed 9 years ago by benDOTsimkinsATintsoftDOTch

Update:

It appears that the problem only occurs if the javascript is the first thing in the html. The following code can be pasted into IE in text mode and doesn't get eaten.

<br /> 
<script type="text/javascript">
  alert('hlleo');
</script>

Curioser and curioser

comment:3 Changed 9 years ago by mharrisonline

The GetHtml plugin preserves JavaScript? in the HTML, the big problem that can occur is if the JavaScript? is supposed to write into the HTML. It will, and will again every time the editor is toggled from HTML view to WYSIWYG view.

comment:4 Changed 9 years ago by anonymous

Please apologize, I already posted this here http://xinha.python-hosting.com/ticket/744 because I missed this ticket.

On the default example page here javascript gets stripped out in IE. Tested with this dummy function:

<script language="text/javascript">
 function foo() {
  alert("bar");
 }
</script>

I also tried with

<script language="text/freezescript">
 function foo() {
  alert("bar");
 }
</script>

Am I doing something wrong or will this work in IE only with Xinha configured to work in FullPage? mode?

comment:5 Changed 9 years ago by wymsy

Here's a modification to setHTML() that works around the problem.

HTMLArea.prototype.setHTML = function(html) {
  if (!this.config.fullPage)
  {
    if(HTMLArea.is_ie) 
    {
      //ie eats scripts at beginning of page, so
      //make sure there is something before the first script on page
      html = html.replace(/( |\s)*<script /," \n<script ");
    }
    this._doc.body.innerHTML = html;
  }
  else
  {
    this.setFullHTML(html);
  }
  this._textArea.value = html;
};

It's kind of a hack, though. It inserts a fixed space before the first script in the editor window. I couldn't find anything better to insert - comments or empty divs, for example, didn't work.

comment:6 Changed 8 years ago by faetze

Hmmmm, I'm afraid that hack doesn't work here.

comment:7 Changed 8 years ago by wymsy

Just noticed that the fixed space entities in the regexp were changed changed to fixed space characters. That line should read:

html = html.replace(/(&nbsp|\s)*<script /,"&nbsp;\n<script ");

(

comment:8 Changed 8 years ago by wymsy

Arrrgh! Make that

html = html.replace(/(&nbsp;|\s)*<script /,"&nbsp;\n<script ");

comment:9 Changed 8 years ago by benDOTsimkinsATintsoftDOTch

Nice to come back and see someone taking care of my problems:)

The wymsy hack is better than nothing I guess, though maybe sub-optimal depending on whether a leading space breaks the layout of what's being edited.
I suggest the following regexp though, so it only catches the first script

html = html.replace(/^<script /," <script ");

comment:10 Changed 8 years ago by wymsy

Ben, my regexp already catches only the first script because it is not global. Yours will miss some situations where the script will be lost, such as empty P tags and many others. The only reliable approach I could find was to force a fixed space before the first script regardless of what else preceded it. I agree it's sub-optimal, though.

comment:11 Changed 8 years ago by jcsalem

This is a decent workaround but I still have a few problems with it:

1) wymsy's regexp doesn't work if the script tag has no attributes (i.e., if there is no space after "<script"). This is easily fixed by removing the trailing space:

html = html.replace(/(&amp;nbsp;|\s)*<script/,"&amp;nbsp;\n<script");

2) In addition to script tags, initial comments are also lost on IE. Also, if there is a comment followed by a script tag (as in the following example) both are lost.
3) As benDOTsimkinsATintsoftDOTch points out, depending on what follows the script tag, this replacement can change the layout. For example, inserting the &amp;nbsp; before "<script":

<script>var foo;</script>
<p>Content</p>

will cause extra whitespace at the top of the page.

Aargh!!!

comment:12 Changed 8 years ago by jcsalem

Oops, the formatting of the first line should be:

html = html.replace(/(&nbsp;|\s)*<script/,"&nbsp;\n<script");

comment:13 Changed 8 years ago by wymsy

I haven't had time to look at this in detail. but it might be possible to move this regexp into inwardHTML(), and add another regexp in outwardHTML() to remove the fixed space. We would have to look at all the places that setHTML() is used - in most cases it is used along with inwardHTML() or outwardHTML(), but there are one or two places where it is used alone - and see if this approach would create any new problems.

comment:14 Changed 8 years ago by wymsy

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

Fixed in changeset:816. Modified the regexp to handle comments as well as scripts. The added nbsp is removed in outwardHtml.

Note: See TracTickets for help on using tickets.