Bug 5404 – fix createMarkup to support XML

• Component: DOM
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

The attached patch fixes createMarkup to better support serializing XML.

[Eric Seidel (no email)]

Created attachment 4386 [details]
Fixes createMarkup's XML support.

This fix is covered at least by XSLT test cases soon to land.

[Eric Seidel (no email)]

Created attachment 4426 [details]
Patch to fix toString() and createMarkup() to better support xml

[Darin Adler]

Comment on attachment 4426 [details]
Patch to fix toString() and createMarkup() to better support xml

I noticed that the doesHTMLForbidEndTag function doesn't do the right thing for
XML.

Otherwise, patch looks pretty good. Need test cases to test all the things
fixed here.

[Eric Seidel (no email)]

Created attachment 4431 [details]
One patched test

[Eric Seidel (no email)]

Created attachment 4468 [details]
Another new test case, using innerHTML

[Eric Seidel (no email)]

Comment on attachment 4468 [details]
Another new test case, using innerHTML

NOTE, view this as source if you want to see what it really looks like.

[Eric Seidel (no email)]

Created attachment 4469 [details]
Patch w/ darin's suggested fixes.

[Maciej Stachowiak]

r- for some issues Eric and I discussed. HTML tags that have no children but aren't assumed empty, such 
as <span>, will serialize without a close tag, this is wrong. I also strongly suggested that XHTML should 
be emitted as HTML-compatible when possible, that is, self-closing tags should be used only for tags 
where an end tag would be forbidden in HTML.

[Darin Adler]

Comment on attachment 4468 [details]
Another new test case, using innerHTML

Clearing review flag from the test case. It's fine, but we don't want this to
show up as a "bug with a reviewed patch".

[Eric Seidel (no email)]

Created attachment 4478 [details]
Patch addressing mjs's compatibility concerns.

[Eric Seidel (no email)]

Created attachment 4479 [details]
Additional innerHTML test cases (both for xhtml and html)

[Darin Adler]

Comment on attachment 4478 [details]
Patch addressing mjs's compatibility concerns.

It's strange to forward-declare a function inline as you do
doesHTMLForbidEndTag and shouldSelfClose. If the thing isn't defined until
after it's used, it won't get inlined, but if it's defined before it's used, it
doesn't need to be forward-declared.

Patch looks fine, r=me.

[Maciej Stachowiak]

+// Rules of self-closure
+// 1. all html elements in html documents close with >
+// 2. all elements w/ children close with >
+// 3. all non-html elements w/o children close with />
+static inline bool shouldSelfClose(const NodeImpl *node)

A few comments on this:

1) I do not think this is a complete statement of the rules. You should also mention that HTML elements 
in an XML document which do not allow a close tag in HTML should use the self-closing syntax.

2) Technically, XML refers to tags like <foo/> as "minimized", not "self-closing", perhaps you would 
like to comply with the official terminology and call this function shouldMinimize (and make other 
corresponding changes.

I'll leave the patch reviewed and leave it to your judgment how to address these remarks.

[Maciej Stachowiak]

Also I think what Darin said about the inliner is wrong in the latest gcc. As long as a function has an inline 
declaration in the same compilation unit, it will get inlined.

[Eric Seidel (no email)]

Fixed!

[Lucas Forschler]

Mass moving XML DOM bugs to the "DOM" Component.