Closed Bug 15674 Opened 25 years ago Closed 25 years ago

[DOGFOOD] Saving to a file loses content

Categories

(Core :: DOM: Serializers, defect, P1)

Product:
Core
Component:
DOM: Serializers
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: unapersson, Assigned: harishd)

References

Details

(Whiteboard: [PDT+])

Attachments

(3 files)

an example of the file before it is opened in Composer
207 bytes, text/html
Details
the same file after opening and saving in Composer (Build ID 1999102110, WinNT)
113 bytes, text/html
Details
A somewhat complicated patch which seems to fix the problem
8.51 KB, patch
Details | Diff | Splinter Review
When you edit a document with a doctype declaration and then save it it replaces all occurances of " with the entity &quot.
An example of a mangled doctype: <!doctype HTML PUBLIC &quot;-//W3C//DTD HTML 4.01 Transitional//EN&quot; &quot;http://www.w3.org/TR/html40/loose.dtd&quot;>
Assignee: buster → akkana
Severity: normal → major
Priority: P3 → P1
Target Milestone: M11
sounds like an output problem. Is the editor forcing the entity mapping, the output converter, the parser?
Status: NEW → ASSIGNED
Depends on: 8865
Accepting bug (also cc'ing Harish, who recently changed doctype handling, but this issue isn't his fault). We're currently depending on code from I18n to decide when to convert to entities, but the code currently does it wrong (i.e. if it can map to an entity, it will, which is clearly wrong for cases like double-quotes). I'm thinking that I'll probably have to stop generating entities at all until 8865 is solved, which would also fix this bug.
Changing Component to Output.
I believe this is a valid bug -- I've seen similar things too, and it's the same issue as in bug 16441 -- but I'm having trouble reproducing this particular problem. Can you give me a specific test case and instructions on how to reproduce this? I tried just putting in the line: <!doctype HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html40/loose.dtd"> but I didn't get the quote substitution.
Try editing an existing document with a doctype declaration, that's how I came across it.
I tried that, and didn't see the problem. Maybe one of the checkins in the last few days also fixed this problem. Try it and see if you can still reproduce it; if you can, then please add the URL to the URL field here, or add the file as an attachment to the bug, and explain how you're invoking the editor on it (e.g. from a file named foo.html, or by browsing to the url then doing edit page, etc.)
Blocks: 16654
It's just got much worse, not sure what has happened. I'm following these actions which triggered the original bug: 1) Open Composer 2) Open File "before.html" (see attachment) by Clicking on Open Icon 3) Make minor textual edit (change "test" to "testing") 4) Save As "after.html" And the result is this: <!> <html> <head> <title>X:²`</title> </head> X:²` <body> <h1>X:</h1> </body> </html>
Assignee: akkana → cmanske
Status: ASSIGNED → NEW
Yes, there's a bad bug in the save code which appeared in the last day or two. It's not in the output code -- if you do Debug->OutputHTML, then everything looks okay and the doctype comes through correctly. It's just when going through the Save code that things don't work. Cmanske has been making changes in the Save code recently; reassigning to him. But unapersson, please try using Debug->OutputHTML and see if you see, as I do, that the doctype is okay now. (There are some spurious newlines in the file ... I'll look at what's causing those -- but that's less serious, there are lots of known bugs with newline/whitespace handling.)
Incidentally, the extra-newlines bug is 17017, if anyone cares; it comes from the parser inserting newlines which were outside the body in the source, inside the body when it gets parsed into the content tree.
My build is current as of 10/20 and I don't see the problem. The work I did in Save code relates to prompting for the title and nothing else -- I don't think has anything to do with it. I opened "before.html", typed and saved it just as described and this is the content of the "after.html": <!doctype HTML PUBLIC &quot;-//W3C//DTD HTML 4.01 Transitional//EN&quot; &quot;http://www.w3.org/TR/html40/loose.dtd&quot;> <html> <head> <title>An Example</title> </head> <body> <h1>Testing</h1> </body> </html> Rick: do you know of any changes in file save code that might have caused this?
Charley: This apparently just appeared in yesterday's build (10/21), so a build from 10/20 wouldn't show it. I think it happens on Windows as well; I seem to recall mjudge asking about it. It's not at the parser/output level, since OutputHTML still works fine. Could this be some change to the file writing code? Adding Simon to the cc list in case he's heard about anyone making changes in file-writing code recently.
I have not landed my changes to XIF yet, so no, I don't think anything changed here to my knowledge.
Charley's right, he didn't change any code recently enough to affect this. I have to assume file spec code, which did change -- dveditz changed it, so I'm guessing him as the owner of the bug. I hear that dougt is also involved with nsFileSpec. Maybe they can figure out why content is being lost when writing to files. Incidentally, this has now been seen on all three platforms (brade sees it on mac, mjudge on windows) so I'm also changing the platform.
How could the file spec code be doing entity substitution? I think dveditz is the wrong owner for this.
Summary: editor mangling doctype declarations → Saving to a file loses content
Sorry, the bug has changed and maybe I should change the summary (doing so); probably I should have closed the old one and opened a new one. The issue is that none of the text node contents make it to the output; the editor's output functions are working, but by the time they get written to disk most of the content is lost.
Assignee: dveditz → dougt
I have no clue about this bug. The only change I made in file spec was to the Copy and Move operations, and this sounds more like streaming.
*** Bug 17480 has been marked as a duplicate of this bug. ***
Severity: major → critical
Summary: Saving to a file loses content → [DOGFOOD] Saving to a file loses content
We're getting a seriously large number of editor bugs filed on this. This needs to be a dogfood issue.
*** Bug 17495 has been marked as a duplicate of this bug. ***
Assignee: dougt → sfraser
I'll look at this asap.
I tried to look at it earlier today to pin it down further, but I was hitting a Javascript error in the editor SaveAs code. :-(
Whiteboard: [PDT+]
Putting on PDT+ radar.
I got past the JS error and did some debugging. The problem comes when writing an nsString to an nsOutputStream. Set a breakpoint in nsHTMLContentSinkStream::Write(const nsString& aString) just after the EncodeToBuffer call. Examine mBuffer (the string which was translated to UTF-8 by EncodeToBuffer). It's fine. But then we do out.write(&mBuffer, mBuffer.Length()); and what gets written out to the stream is garbage.
Assignee: sfraser → akkana
Aha! When Harish changed mBuffer from a char* to an nsCAutoString, he didn't change where Write was sending &mBuffer to a method that expected char*, and I didn't catch it in my review. Harish and I are making a fix.
Assignee: akkana → harishd
Aha, it wasn't my review after all. ;-) Turns out there was Solaris build bustage which caused the & to get put back in; my suggestion is that the right solution is probably to replace the & with a (char*) cast. Assigning to Harish.
Changing the & to a (char*) should fix the problem -- but it doesn't, there's some problem in the way the unicode encoder is encoding to the nsCAutoString. I had another issue I was working on, changing to use nhotta's new SaveAsCharset service to do unicode and entity encoding; those changes combined with getting rid of the & does fix the problem, at least for me. I've attached the patch in case anyone who's being held up by this bug wants to apply it, while Harish and Naoki and I go over it to make sure it's kosher for other charsets.
*** Bug 17515 has been marked as a duplicate of this bug. ***
adding myself to cc: list - affects mail compose as well.
Status: NEW → RESOLVED
Closed: 25 years ago
Resolution: --- → FIXED
Should be fixed.
Blocks: 15017
Harish/unaperson, can you verify this and mark verified-fixed? thanks!
Status: RESOLVED → VERIFIED
verified in 11/29 build. Thanks Kathy for the easy steps.
Bulk move of all "Output" component bugs to new "DOM to Test Conversion" component. Output will be deleted as a component.
Component: Output → DOM to Text Conversion
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: