45627 - XMLSerializer::serializeToString() doesn't recognize empty elements

Status: VERIFIED FIXED

(Core :: XML, defect, P3)

Description

[Taras Tielkes]

var xmlDoc = document.implementation.createDocument("", "x", null);
alert( new XMLSerializer().serializeToString( xmlDoc ) );
-------------------------------------------------------------
This will show "<x></x>".

This is absolutely correct, but it would be nice if XMLSerializer would 
recognize that an element is empty, and output an empty element tag. This will 
save some space with larger documents with a lot of empty elements.(Trivial 
though, but looks nice)

Comment 1

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

I have a fix, will attach.

Comment 2

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

Attachment: Proposed fix, elements with no children output only start tag

Comment 3

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

Attachment: testcase 1

Comment 4

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

Attachment: Patch 1/2 (extensions/base)

Comment 5

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

Attachment: Patch 2/2 (layout/base)

Comment 6

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

Attachment: Patch 3/3 (yeah yeah, wrong numbers) (layout/build)

Comment 7

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

jst pointed out that we already have content serializers in layout. I removed
code duplication by calling the implementation in layout dir. While doing this I
noticed several bugs in the implementation there, namely:

- empty elements are not output as empty
- HTML tags get uppercased
- default namespace somehow gets assigned the prefix "xmlns" for output
- HTML tags get the HTML namespace declaration attribute twice
- script tag contents do not get output in testcase 1

My patches 1/2, 2/2 and 3/3 applied together fix all but the last two problems.
They are still suboptimal (too many string operations to my liking, and if
someone actually declared namespace prefix "xmlns" we would not output that).

This fix also fixes bug 45552.

Comment 8

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

I am taking all XMLExtras bugs to make it easier to see what I am working on...

Comment 9

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

PIs seem to be missing as well.

Comment 10

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

Attachment: testcase 2: XMLHttpRequest + parser + serializer

Comment 11

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

Attachment: New patch to layout/base

Comment 12

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

Attachment: HTML Namespace fixes, re-enable ConfirmPrefix

Comment 13

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

The last patch to layout/base enables ConfirmPrefix again. Now I know the
purpose: it is needed in case some script was moving elements around, for instance.

Also, because HTML elements do not really understand namespaces, they do not
report the namespace prefix properly - rather the name contains the prefix and
the actual name. There are quite a few hacks to work around this.

I have opened new bugs to track the issues that I have found while working with
this:

- bug 63559 DOMParser loses HTML element attributes
- bug 63558 XMLSerializer does not serialize starting XML PI
- bug 63560 XMLSerializer does not output html:script contents
- bug 63561 XMLSerializer does not output comments and PIs after document
element close

Comment 14

[vidur (gone)]

r=vidur. We should get the document encoder to work on free-standing subtrees as
well.

Comment 15

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

Bug 63568 opened to track the issue of serializing free-standing subtrees.

Comment 16

[Johnny Stenback (:jst)]

sr=jst for the patches 1/2, 2/2 and the last patch.

I have one coding style nitpick about the last patch, code like this:

  if (!content) return NS_ERROR_FAILURE;

I'd prefer seeing that either either broken up on two lines or:

  NS_ENSURE_TRUE(content, NS_ERROR_FAILURE);

... but I'd accept the patch as is if you don't wanna change that.

I think the namespace code could be cleaned up alot once our HTML elements use
nsINodeInfo so that prefixes and namespaces are always correctly preserved.

Comment 17

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

Fixed.

Comment 18

[Chris Petersen]

Marking verified per last comments.