User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 If a web server that is serving a page shuts down, or if the http connection gets reset, a </script> may not be sent to the browser. Firefox should then ignore the <script> block. It SHOULD NOT parse the data as HTML! e.g. <script><!-- var x = '><iframe src="http://www.google.com"></iframe>'; This bug is VERY bad, because random web pages may get requested. And there doesn't seem to be a way from preventing the browser from processing the data as HTML if the end-script isn't sent. Reproducible: Always Steps to Reproduce: 1. Open the HTML shown above. (I'll attach the test case as well) 2. 3. Actual Results: The iframe is shown. Expected Results: The page should be blank. The script data should be ignored. That's what IE does.

I always thought we did this for compat with IE, but investigating shows that IE actually consumes <script> and <style> non-conservatively (i.e., it eats to the end of the document if we don't find the closing </script> or </style>). However, IE refuses to execute such scripts. Should we follow IE here? Opera does what we do. I'm not sure about other browsers.

Which IE did you test and in standards or quirks mode? ;)

This was IE6 on Windows. The behavior is the same in both standards and quirks mode.

Huh. Do our cvs logs have any indication why we're doing things this way?

The logs blame bug 35456, where harishd says that he implemented the current behavior for compatibility with Nav4.x, but when I tested with 4.8, I saw really weird behavior (a random character appeared on the page that went away when I tried to select it). So it looks like the only sticking points for emulating IE are: Opera's behavior (do we care?) and that we shouldn't run the script if there is a malformed or missing </script> tag.

Confirming based on bug 311395.

Comment on attachment 208419 [details] [diff] [review] Fix this won't cancel enough serialization

The viewPartialSource.js fixes are really hacks, but they work and they produce the "right" output. If anybody actually does use DOM2 elements to append elements to a document that has an unclosed <script>, they get what's coming to them!

Comment on attachment 209903 [details] [diff] [review] "Fix" serialization >Index: content/base/src/nsHTMLContentSerializer.cpp >+static PRBool >+HasNextSibling(nsIContent *aContent) "sibling" isn't really used right here. Alternative names: HasNextInDocumentOrder, HasFollowingInDocumentOrder, ContentOrParentHasFollowingSibling >@@ -724,36 +751,59 @@ nsHTMLContentSerializer::AppendElementSt > SerializeAttributes(content, name, aStr); > > AppendToString(kGreaterThan, aStr); > > if (LineBreakAfterOpen(name, hasDirtyAttr)) { > AppendToString(mLineBreak, aStr); > mMayIgnoreLineBreakSequence = PR_TRUE; > mColPos = 0; > } > >+ if (name == nsHTMLAtoms::script) { >+ // Handle malformed script serialization. In general, we don't want to >+ // serialize end tags for scripts, so that round-tripping a document that "serialize end tags of malformed scripts" >+ // doesn't have a </script> won't suddently start executing scripts. >+ // However, if someone was evil, and appended elements (using DOM2 methods) What's wrong with DOM1 or DOM0 methods :) just say "DOM"

Comment on attachment 209915 [details] [diff] [review] Updated to comments >Index: toolkit/components/viewsource/content/viewPartialSource.js ... >+ var insertEndMarker = true; >+ var tmpNode = ancestorContainer; >+ >+ if (tmpNode instanceof HTMLScriptElement) >+ tmpNode = tmpNode.parent; >+ >+ while (tmpNode) { >+ if (tmpNode.lastChild instanceof HTMLScriptElement) { >+ var inner = tmpNode.innerHTML; >+ if (inner != "<script>" && inner.charAt(inner.length - 1) != '>') { >+ insertEndMarker = false; >+ } >+ >+ break; >+ } >+ tmpNode = tmpNode.lastChild; >+ } You can change this to while (tmpNode) { if (tmpNode instanceof HTMLScriptElement && tmpNode.parentNode) { var inner = tmpNode.parentNode.innerHTML; if (inner != "<script>" && inner.charAt(inner.length - 1) != '>') { insertEndMarker = false; } break; } tmpNode = tmpNode.lastChild; } To avoid the extra tmpNode = tmpNode.parentNode r=me either way.

We went back in time to my original serialization behavior. Ugh, here's hoping that this is the final patch.

Comment on attachment 210195 [details] [diff] [review] With different roundtripping sr=jst

Fix checked into trunk.

So with this patch we have IE compat and not Opera/Konqueror compat, right? I think that's fine, but perhaps we should file equivalent bugs on them? And get Hixie to spec this out in HTML5?

Filed one on Opera.

I filed bug 327796, "Unclosed scripts with src attribute should be allowed to run". Anne and Hixie, since you're involved in speccing this and/or trying to ensure cross-browser consistency, you might be interested.

Comment on attachment 210195 [details] [diff] [review] With different roundtripping Is this one worth fixing for FF2? This particular patch changes the nsIScriptElement interface, though, so a 1.8 patch might not be worth the effort.

Comment on attachment 210195 [details] [diff] [review] With different roundtripping I'm not sure if it's worth the effort, but if someone wants to do it please go ahead. a=me if so.

*** Bug 362397 has been marked as a duplicate of this bug. ***

http://developer.mozilla.org/en/docs/Updating_web_applications_for_Firefox_3#Change_to_the_SCRIPT_element Now documented.

You may find the following commentary interesting: http://slashdot.org/~einhverfr/journal/203094 Maybe others can request that this change be made too.

See bug 327796 and bug 408702.