1363658 - Serializing DocumentType (doctype) nodes should not include the internal subset

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Philip Jägenstedt [:foolip]]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.96 Safari/537.36

Steps to reproduce:

https://github.com/w3c/web-platform-tests/pull/5648

This PR added this test:
https://github.com/w3c/web-platform-tests/blob/master/domparsing/DOMParser-parseFromString-xml-internal-subset.html
http://w3c-test.org/domparsing/DOMParser-parseFromString-xml-internal-subset.html


Actual results:

assert_equals: serialized doctype expected "<!DOCTYPE foo>" but got "<!DOCTYPE foo [ <!ENTITY x \"y\"> ]>"


Expected results:

This test should pass, because the internal subset isn't mentioned in https://w3c.github.io/DOM-Parsing/#xml-serializing-a-documenttype-node

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Henri would probably know what goes run in our code. :)

Comment 2

[Henri Sivonen (:hsivonen)]

https://searchfox.org/mozilla-central/source/dom/base/nsXMLContentSerializer.cpp#368

We really should should rewrite our serializers in Rust to get rid of a whole bunch of stuff that's meant for editors and is unnecessary for a browser. The editor-oriented stuff should really live in the BlueGriffon tree instead of m-c.

Comment 3

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

Attachment: Bug 1363658 - Serializing doctypes should omit internal subset;

Changes to match spec, Chrome, and Safari.  The spec was discussed and
is what we want -- we already expand entities from the internal subset
when parsing, so there's no need to remember their definitions.  Indeed
it seems like it would make sense to alter the parser to throw away the
internal subset entirely at the end of parsing.

Review commit: https://reviewboard.mozilla.org/r/164474/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164474/

Comment 4

[Henri Sivonen (:hsivonen)]

Comment on attachment 8893372 [details]
Bug 1363658 - Serializing doctypes should omit internal subset;

https://reviewboard.mozilla.org/r/164474/#review170160

::: commit-message-52285:7
(Diff revision 1)
> +
> +Changes to match spec, Chrome, and Safari.  The spec was discussed and
> +is what we want -- we already expand entities from the internal subset
> +when parsing, so there's no need to remember their definitions.  Indeed
> +it seems like it would make sense to alter the parser to throw away the
> +internal subset entirely at the end of parsing.

Please file a follow-up to that effect. Has the DOM spec been changed not to expose the internal subset?

Comment 5

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

The DOM spec hasn't contained internalSubset for ages:

https://dom.spec.whatwg.org/#dom-documenttype-internalsubset

And we removed it in bug 801545 close to two years ago.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3594bfc41c25
Serializing doctypes should omit internal subset; r=hsivonen

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3594bfc41c25