Bug 277440 – XMLSerializer.serializeToString() not serializing child(s) of <img> and also not closing the <img> if it has child(s)

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Deepak Aravind]

<img> tag is not closed in serialization through XMLSerializer if <img> tag has child(s), this is different from chromium as it is serializing the <img>'s children.


This can be reproducible 

const root = document.createElement("div");
const newImage = document.createElement("img");
const newStyle = document.createElement("style");

newImage.appendChild(newStyle);

root.appendChild(newImage);
document.body.appendChild(root);


const serializer = new XMLSerializer();

const serialized = serializer.serializeToString(root)

console.log(serialized);


Output:

Chrome: 
<div xmlns=\"http://www.w3.org/1999/xhtml\"><img><style></style></img></div>

Safari:
<div xmlns=\"http://www.w3.org/1999/xhtml\"><img></div>

[Deepak Aravind]

Source/WebCore/editing/MarkupAccumulator.cpp#260

MarkupAccumulator::serializeNodesWithNamespaces, here if the <img> has child nodes the self tag would be skipped since shouldSelfClose will be false thus self close tag is not added,
```
void MarkupAccumulator::appendCloseTag(StringBuilder& result, const Element& element)
{
    if (shouldSelfClose(element, m_serializationSyntax)) {
        if (element.isHTMLElement())
            result.append(' '); // XHTML 1.0 <-> HTML compatibility.
        result.append('/');
    }
    result.append('>');
}
```

but `elementCannotHaveEndTag` this been checked for to add close tag (</img>) but this would always fails regardless of whether node has children.

So if `shouldSelfClose` fails we should add closeTag

[Deepak Aravind]

The shouldSelfClose and elementCannotHaveEndTag were contradicting each other where i believe both return opposite values but as per current implementation  producing same result

const myImg = document.createElement("img")
const childDiv = document.createElement("div")
img.appendChild(childDiv)


Consider current, targetNode = myImg
From the above case, 
shouldSelfClose returns false as it has child(s)
elementCannotHaveEndTag returns also false as it would always return false for void-elements



We have to make a decision clearly here,
1.Allow children in void elements (as per spec, it is incorrect, but chrome doing this)

2.shouldSelfClose would give priority for whether it is a void element immediately return true over hasChildNodes

3.Don't allow (or Ignore) appending node on void elements

[Deepak Aravind]

In the above am referring shouldSelfClose and elementCannotHaveEndTag are implementation which are located in Source/WebCore/editing/MarkupAccumulator.cpp

[Radar WebKit Bug Importer]

<rdar://problem/133404338>

[Anne van Kesteren]

Per https://w3c.github.io/DOM-Parsing/#the-xmlserializer-interface it should always produce an XML serialization. In the XML world concepts such as "shouldSelfClose" and "elementCannotHaveEndTag" are not applicable as all elements are equivalent when it comes to syntax.

So 1 is the correct option from the list in comment 2 except that it is per specification and not against the specification.

Hope that helps.

[Deepak Aravind]

Just making sure, we planning to allow constructing serialization for children of void elements. And you are saying it is correct as per spec?

[Anne van Kesteren]

Yes, that is correct. XML (just like the DOM, but unlike HTML) does not have a concept of void elements. Every element can have child elements. So when you serialize as XML, you essentially have to ignore HTML-specific serialization aspects.

[Deepak Aravind]

Agree with this ! 
Thanks a lot for the answer, i will try to raise a PR for this.
Thanks again

[Deepak Aravind]

Pull request: https://github.com/Webkit/WebKit/pull/32528

[Deepak Aravind]

Pull request: https://github.com/Webkit/WebKit/pull/32628

[Deepak Aravind]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/47775

[EWS]

Committed 282725@main (0afcbcac01bd): <https://commits.webkit.org/282725@main>

Reviewed commits have been landed. Closing PR #32628 and removing active labels.