207904 - Saving XML saves wrong contents !

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[beanladen]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030529
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030529

Saving the file given via right button changes the contents of the file saved.
Line 6 is changed from <!DOCTYPE  SearchResuls> to <!DOCTYPE  [earchResul]> !


Reproducible: Always

Steps to Reproduce:
1.Goto given URL
2.Save via right button 'save link target as...'
3.View contents of saved file and source view of file

Actual Results:  
Line 6 is changed from <!DOCTYPE  SearchResuls> to <!DOCTYPE  [earchResul]> !

Expected Results:  
Do not change downloaded file contents !

Very strange thing, I've never seen something like this before.
Maybe there's more changed, I did not check the whole contents.

Comment 1

[beanladen]

Severe enough to block 1.4 ?

Comment 2

[beanladen]

Of course, it should be <!DOCTYPE  SearchResult> ....
Active character coding is UTF 8.

Comment 3

[Bill Mason]

WFM 20030529 PC/WinXP

Comment 4

[Boris Zbarsky [:bzbarsky]]

You're saving the page as "Web Page, complete", so we do various fixup of the
data to point urls to relative forms of the urls.

We shouldn't be changing the doctype like that, though....

Comment 5

[Adam Lock]

I don't see anything in the persist object that might cause this problem,
however "save as complete" also involves a parser, a DOM and XML serializer so
it could be a problem with one of those. Saving only the page just dumps out the
unmolested original data straight from the URI.

In particular I notice from breakpointing here:

http://lxr.mozilla.org/seamonkey/source/content/base/src/nsXMLContentSerializer.cpp#225

That the doctype is saved with no name and internalSubset == "earchResul". So
looking further and I see the parser is at fault. During parsing it has lopped
off the start and end characters assuming them to be '[' and ']', stuffing the
rest into the internalSubset value.

I think <!DOCTYPE SearchResult> is valid XML from the spec, but perhaps the
parser is not doing the right thing when is not followed by a public or system
keyword or a set of declarations.

Comment 6

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

This a regression. I don't know yet when this regressed, but something changed
so that |mDoctypeText| no longer includes the closing |>| which throws off
|GetDocTypeToken()|.

Comment 7

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

Attachment: testcase

Comment 8

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

Attachment: Proposed fix

There have been same changes in how we handle the [] delimiters in the internal
subset, but I didn't notice what could have caused the difference.

I noticed in my testing that the doctype closing '>' is no longer in
mDoctypeText, therefore it is no longer a delimiter in |GetDocTypeToken()|. To
fix the case where  there is nothing but name, I decided to test if the
delimiter find returned a negative value, and in that case make the method
consume the whole doctype.

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 124776 [details] [diff] [review]
Proposed fix

sr=jst

Comment 10

[harishd]

Comment on attachment 124776 [details] [diff] [review]
Proposed fix

r=harishd

Comment 11

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

Checked in on trunk, leaving open to get this to the branch.

The fix is low risk, fixes a dataloss regression issue that actually prevents
you from opening the XML files in Mozilla (because they will become invalid XML).

Comment 12

[Rafael Ebron (:rebron)]

a=adt.  Please land this fix on the Mozilla 1.4 branch and add fixed1.4 to the
staus whiteboard.

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 124776 [details] [diff] [review]
Proposed fix

a=asa (on behalf of drivers) for checkin to the 1.4 branch.

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 124776 [details] [diff] [review]
Proposed fix

Please add fixed1.4 when checked into branch

Comment 15

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

Fixed on branch as well.

Comment 16

[Madhur Bhatia]

verified fixed on branch 
checked builds :-
Linux 2003-06-19-07-1.4
WinXP 2003-06-24-08-1.4

changing KW : fixed1.4 to verified1.4

keeping the status as resolved for now, since it has to be verifed fixed on
trunk as well.