Bug 3501

Created attachment 2330 [details] Possible solution (tested and found working by Joost de Valk) The following patch is taken straight from KHTML, which is not affected by this bug. Instead of look at the TEXT attribute, the text is now extracted from the TEXT child.

Comment on attachment 2330 [details] Possible solution (tested and found working by Joost de Valk) This doesn't handle the case where the <script> contains multiple text nodes, or text nodes and cdata nodes, or text nodes and cdata nodes and comment nodes and element nodes and entity nodes. It should probably be a little cleverer. :-)

Konqueror handles the following testcases just fine: CDATA, multiple nodes and mixed nodes types. http://www.rakaz.nl/projects/safari/showscripts.html Did you come to your conclusion after testing the patch, or just from looking at the patch?

Created attachment 2332 [details] Extended testcase This test case will also test for multiple nodes and mixed node types.

At this time the patches won't build.

Comment on attachment 2330 [details] Possible solution (tested and found working by Joost de Valk) JdV: works for me :)

Those two testcases are sent as text/html and therefore do not test the XML case that I was referring to. (There are no comments or CDATA blocks in those testcases, since they are parsed as HTML, in which the parser treats the entire <script> element as having a single CDATA block.) I'll change the testcase attached to this bug to have the right MIME type. Test it again and you should see that it in fact doesn't work. When running that test as XML you should see, for the .text attribute of each <script> element in the file: 1. Two blank lines for the first <script> block ("\n\n"). (And indeed the entire test won't run when you send it as XML since the script that runs the test is commented out.) 2. Two blank lines for the second <script> block. 3. The following for the third: "\n\nfunction noname2(){\n\t/*script*/\n\n}\n" 4. For the fourth: "\nfunction noname3(){\n\t/*script*/\n}\n" 5. Then: "\n\n\n" 6. The sixth is the following two strings concatenated (I couldn't fit them on one line here): "\n\nfunction noname5a(){\n\t/*script*/\n}\n" "\n\nfunction noname5b(){\n\t/*script*/\n}\n" 7. And finally: "\n\n\nfunction noname6b(){\n\t/*script*/\n}\n\n" ...where of course "\t" is a tab character and "\n" is a newline character. Also note that "document.getElementsByTagName("SCRIPT");" shouldn't work in XHTML, you'll have to do this instead: document.getElementsByTagNameNS("http://www.w3.org/1999/xhtml", "script") Namely, you have to use lowercase and you have to use the *NS variant and give the namespace.

I've replaced the text() function with the following snippet() and it seems to work, with one exception: <script> <[CDATA[ ... ]]> </script> The newline between <script> and <[CDATA[ is not included. Neither is the newline between ]]> and </script>. Those newlines should be of node type TEXT_NODE, so the most probably cause is that they are stripped elsewhere, because they only contain whitespace. DOMString HTMLScriptElementImpl::text() const { if (firstChild()) { QString scriptCode = ""; NodeImpl *child; for (child = firstChild(); child; child = child->nextSibling()) { if (child->nodeType() == Node::TEXT_NODE || child->nodeType() == Node::CDATA_SECTION_NODE) { scriptCode += child->nodeValue().string(); } } return scriptCode; } return ""; } I still need to look at the other function. Replacing the text also becomes more complicated, because we may need to replace multiple nodes with a single text node...

Created attachment 2368 [details] XHTML and HTML compatible patch A modified patch that is compatible with both documents that are parsed by the XML parser and documents parsed by the HTML parser. XML behavoir is tested with: http://www.rakaz.nl/projects/safari/showscripts.php HTML behavoir is tested with: http://www.rakaz.nl/projects/safari/showscripts.html

Looks like the fix I did for 3256 fixes this as well (except for changing HTMLScriptElement::text() and setText() to call the implementation)

This bug can be closed, the fix suggested in this bug is incorporated into the patch for bug 3526.

*** This bug has been marked as a duplicate of 3526 ***