44564 - XML DOCTYPE content not rendered correctly in page source window.

Status: VERIFIED FIXED

(Core :: XML, defect, P3)

Description

[Chris Petersen]

Build: 2000070311
Platforms: All
Expected results: Page source window should display entire content of !DOCTYPE 
including element and entity declarations.
What I got: The first character of ELEMENT and ENTITY declaration are not 
appearing in page source window.
Instead of this:

<!DOCTYPE TEST [
<!ELEMENT TEST (#PCDATA)>
<!ENTITY CP "Chris Petersen">
]>

I get:

<!DOCTYPE TEST [
<!- LEMENT TEST (#PCDATA)>
<!- NTITY CP "Chris Petersen">
]>

Steps to reproduce:

1) Open attached test case.
2) The well-form xml is rendered.
3) Choose Page Source from the View menu.
4) The first character in ELEMENT and ENTITY declarations are missing.

Comment 1

[Chris Petersen]

Attachment: A simple xml test with a internal DTD defined

Comment 2

[Nisheeth Ranjan]

This bug has been marked "future" because the original netscape engineer working
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way
-- please attach your concern to the bug for reconsideration.

Comment 3

[Nisheeth Ranjan]

*** Bug 47488 has been marked as a duplicate of this bug. ***

Comment 4

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

*** Bug 55844 has been marked as a duplicate of this bug. ***

Comment 5

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

I hate this, taking.

Comment 6

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

Attachment: testcase 2: XML with various DTD constructs and serializer

Comment 7

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

Attachment: testcase 3: HTML with various DTD constructs and XML serializer

Comment 8

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

Here is how testcase 2 looks like in view source.
 * incorrect markup declaration open
 * incorrect cdata section close

<?xml version="1.0"?>
<!DOCTYPE x PUBLIC "-//Heikki Toivonen//DTD Foobar//EN" "foo" [

<!- LEMENT x ANY>
<!- TTLIST x id ID #IMPLIED foo CDATA "ab>oo">
<!-- A comment -->
<!- OTATION hep PUBLIC "-//Heikki Toivonen//NOTATION hep//EN">
<!- NTITY nop "no op">
<!- NTITY % param "foo">
]>
<x>
<![CDATA[
foobar
]]]]>
<foo>&nop;</foo>
<html:script xmlns:html="http://www.w3.org/1999/xhtml">
setTimeout("dump(new XMLSerializer().serializeToString(document))",1000);

</html:script>
</x>

This is how it looks like serialized:
  * no <?xml ... (this is another bug)
  * comment moved from internal DTD subset to the beginning (empty line where it
used to be)... this is what the DOM looks like

<!-- A comment --><!DOCTYPE x PUBLIC "-//Heikki Toivonen//DTD Foobar//EN" "foo" [
<!ELEMENT x ANY>
<!ATTLIST x id ID #IMPLIED foo CDATA "ab>oo">

<!NOTATION hep PUBLIC "-//Heikki Toivonen//NOTATION hep//EN">
<!ENTITY nop "no op">
<!ENTITY % param "foo">
]>
<x foo="ab&gt;oo">
<![CDATA[
foobar
]]>
<foo>no op</foo>
<html:script xmlns:html="http://www.w3.org/1999/xhtml">
setTimeout("dump(new XMLSerializer().serializeToString(document))",1000);
</html:script>
</x>

You can test with testcase 3 as well and it will give similar results, even
though the serialializer outputs funny stuff because it is an HTML document and
an XML serializer.

Comment 9

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

Attachment: Proposed fix 1

Comment 10

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

Proposed fix 1 is my first cut at fixing this. I introduced a new token,
(markup) declaration, so that we will no longer treat them as comments and eat
the beginnings of the declarations. I also fixed a couple of static global
variables, and removed the virtual destructor from nsIDTD interface (interfaces
must not have virtual destructors, see bug 31750).

Now view source looks like:

<?xml version="1.0"?>
<!DOCTYPE x PUBLIC "-//Heikki Toivonen//DTD Foobar//EN" "foo" [

<!ELEMENT x ANY>
<!ATTLIST x id ID #IMPLIED foo CDATA "ab>oo">
<!-- A comment -->
<!NOTATION hep PUBLIC "-//Heikki Toivonen//NOTATION hep//EN">
<!ENTITY nop "no op">
<!ENTITY % param "foo">
]>
<x>
<![CDATA[
foobar
]]>
<foo>&nop;</foo>
<html:script xmlns:html="http://www.w3.org/1999/xhtml">
setTimeout("dump(new XMLSerializer().serializeToString(document))",1000);

</html:script>
</x>

And serialized view has not changed.

Comment 11

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

I opened bug 64448 to track the issue of comments moving out of the internal subset.

Comment 12

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

Attachment: Proposed fix 2

Comment 13

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

Poposed fix 2 contains also the proposed fix to bug 64448. Additional
improvements from the first patch are:

- better handling of > embedded in quotes in markup declarations
- add handling of the eToken_declaration to the various DTDs, without this the
HTML DOM (when we have an HTML document with internal subset) is missing these
items... with the patch they appear as comment nodes
- faster recognition of the markup declaration tokens

Comment 14

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

harishd did some reviewing, here are his comments:

- avoid assigning to mText in token creation, only do this on demand
(GetStringValue())... it looks like the assignment in constructor is not
normally used, but I will test this
- do not use eHTMLTag_markupDecl, create a new one for these eToken_declarations
- do not strip \r in Consume and remember line number... fix it to use sliding
strings etc. to avoid copying
- try to enable syntax colored view source (make it a pref)

Comment 15

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

Attachment: Proposed fix 3

Comment 16

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

Harish, I have fixed all the things you noted. Precheckin tests pass (well, I
haven't been able to use mail in Mozilla in a long time and I believe somebody
broke FTP). Because I touched normal HTML code I would feel much better if we
could run the regression tests. Provided they pass, could you give an r?

Comment 17

[harishd]

Patch looks good to me. r=harishd

Comment 18

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

And parser regression tests pass.

Comment 19

[vidur (gone)]

sr=vidur.

Comment 20

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

Fix checked in.

Comment 21

[Chris Petersen]

Verified fixed in the Feb 05 build (2001020513).