63560 - XMLSerializer does not output html:script contents

Status: VERIFIED FIXED

(Core :: XML, defect)

Description

[Heikki Toivonen (remove -bugzilla when emailing directly)]

html:script element will always be empty when it is serialized.

Comment 1

[Heikki Toivonen (remove -bugzilla when emailing directly)]

If you save this document as test.xml it will output empty script element:

<script xmlns="http://www.w3.org/1999/xhtml">
setTimeout("dump(new XMLSerializer().serializeToString(document))",1000);
</script>

Now if you save it as test.html, you will get the correct script element
contents. So it looks like we loose the script contents somewhere in XML path
only. The mChildren of the script element is empty in XML document case while
there are children in HTML document case.

Comment 2

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Ok, found the place. In nsXMLContentSink::ProcessEndSCRIPTTag() we have
FlushText(PR_FALSE);. This means that the text of the script contents are not
added to the DOM (even though the script is run). Calling FlushText(); fixes
this. There is an interesting side effect: if the test document only contains
the script element, the contents are now displayed in the document. If there are
additional elements (or maybe it is just a root element thing), it is not visible.

We also need to take care of the situation where the script element contents are
inside a CDATA section.

I'll attach a wip for fix. wip because I am seeing a lot of warnings and even an
exception in JS code in the console. The wip also contains some small other
fixes in dom and expat and the content sink.

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: WIP for fix 1

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Testcase: script with several CDATA and normal section

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Proposed fix, seems to work correctly with multiple/mixed CDATA/text sections

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I no longer see any problems reported on the console, and everything seems to be
working fine.

I changed some booleans in the content sink class to packed bools, which saves 4
bytes (yeah, like it matters), but I also needed to intruduce a new string
member that holds the contents of a currently open script tag as text. This was
needed to account for the case where a script tag could contain several CDATA
sections as well as normal text sections. The testcase has a situation like
this. I also found an unused define, which I removed. The patch contains also a
fix to doctype handling - the XML parser has already detected if there were
multiple doctypes so we do not need to worry about that in XML.

The fixes in expat dir are two fix compiler warnings, and the fixes to dom to
enable the method "initMouseEvent".

Comment 7

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Avoid (possibly an allocation and) a string copy

Comment 8

[Heikki Toivonen (remove -bugzilla when emailing directly)]

harishd says r.

Comment 9

[harishd]

r=harishd

Comment 10

[vidur (gone)]

sr=vidur for all but the nsDOMPropEnums/nsDOMPropNames change (which shouldn't
be necessary). Also, the compiler warnings fixed in xmlparse.c could be fed back
into the original (SourceForge-based) repository.

Comment 11

[Heikki Toivonen (remove -bugzilla when emailing directly)]

The last patch (minus changes in the dom directory) checked in yesterday.
Marking fixed.

Comment 12

[Chris Petersen]

Marking verified per last comments.