evalScripts and commented JS produeces error
Reported by Evil.2000 | May 8th, 2008 @ 06:27 PM | in 1.6.1
If one page loades a second page with
Ajax.Updater(
"content_id",
"document.html",
{evalScripts: true}
);
and you have the following script in document.html
<script type="text/javascript">
<!--
your_script_code();
//-->
</script>
then the IE produces a JavaScript syntax error in prototype.js on line 597 (see image attached).
Firefox and Opera are executing the script without problems.
If the comments are removed the script will work in IE too.
Comments and changes to this ticket
-
Henrik Hjelte July 3rd, 2008 @ 12:00 PM
- no changes were found...
-
Henrik Hjelte July 3rd, 2008 @ 03:51 PM
- no changes were found...
-
John-David Dalton July 7th, 2008 @ 06:24 AM
- → Milestone changed from “” to “1.6.1”
- → State changed from “new” to “bug”
- → Title changed from “ajax.updater: evalScripts and commented JS produeces IE error” to “evalScripts and commented JS produeces error”
- → Assigned user changed from “” to “John-David Dalton”
This has been addressed via:
http://dev.rubyonrails.org/ticket/11423
It is not going to make it into the 1.6.0.3 release.
We also need to discuss where to place the exec method and maybe even a name change for it.
-
Anders K July 3rd, 2008 @ 10:58 AM
- → Tag changed from “” to “ajax comments evalscripts ie needs_patch needs_tests”
I have seen this bug as well...
...and solved it by updating the regexp that parses the document for script tags.
We are working on submitting a patch with this fix included
-
Henrik Hjelte July 3rd, 2008 @ 12:00 PM
Ok, here is the patch. We have added some new tests to testExtractScripts and testEvalScripts, they pass on Ie6, Ie7, FF2 and Safari 3.
Note that the new Emacs js2-mode has stripped some unnecessary whitespace as well.
/Anders Karlsson and Henrik Hjelte
-
John-David Dalton July 3rd, 2008 @ 03:07 PM
Can you please produce a patch that isn't cluttered with whitespace mods.
I also dont see any real code change). Is this the correct patch?
-
Juriy Zaytsev July 3rd, 2008 @ 03:46 PM
Yes, something is up with this patch. The test additions are:
+ ('ie problem <script><!--\nevalScriptsCounter++\n//--><'+'/script>fixed').evalScripts(); + this.assertEqual(5, evalScriptsCounter);There are no string.js (and String#evalScripts) changes though.
-
-
Henrik Hjelte July 3rd, 2008 @ 03:53 PM
Jurly: There are more tests, but the fix is only with the regexp in prototype, so string.js didn't need any changes.
-
John-David Dalton July 7th, 2008 @ 06:23 AM
Also remember that the ticket I link to at the top fixed this issue by using a global eval solution.
-
Henrik Hjelte July 7th, 2008 @ 02:32 PM
Yes, but they don't need to exclude each other. The arguments for applying this patch:
- works now
- is a clean one-line bugfix
- has testcases
- also fixes problems with extractScripts
- won't cause problems with the global eval solution
-
Henrik Hjelte July 8th, 2008 @ 04:59 PM
Yes, you're right, we started with the patch against 1.6.0.2, and didn't see that the RegExp was updated.
Please Login or create a free account to add a new comment.
You can update this ticket by sending an email to from your email client. (help)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
The Prototype JavaScript library.
