Closed Bug 343870 Opened 16 years ago Closed 15 years ago

Line breaks normalized twice

Categories

(Core :: XML, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha6

People

(Reporter: hsivonen, Assigned: peterv)

References

()

Details

Attachments

(3 files, 1 obsolete file)

Content sink & expat driver tamper with line breaks that come from expat. This is wrong. It is the job of the XML processor to normalize line breaks. However, if there is 
 in the source, it should be expanded as a CR in the DOM and the DOM builder shouldn't normalize it to LF.

Steps to reproduce:
1) Load http://hsivonen.iki.fi/test/newline-normalization.xhtml

Actual results:
The last test fails.

Expected results:
The four cases should all pass.
Taking.
Status: NEW → ASSIGNED
Assignee: xml → hsivonen
Status: ASSIGNED → NEW
Status: NEW → ASSIGNED
Rumor has it that bug 18333 needs to be fixed first.
Depends on: incrementalxml
For the record, it sounds like this bug would have to be fixed in order for the folks clamoring for the ability to add line breaks to tooltips to get their wish (I'm not necessarily one of them, mind you), at least when writing XML.  For details, see bug 358452.  That bug is written in the context of HTML, but should apply just as much to XHTML (a fix for this bug would play the same role as an ideal fix for bug 322270 in the HTML context).
This has pretty much nothing to do with bug 358452, as far as I  can tell.

We should probably just fix this.  It'll basically involve removing that code in the expat driver and nothing else, right?
Flags: blocking1.9?
Blocking since we have to give peterv some blocker too, he'd feel left out otherwise :)
Assignee: hsivonen → peterv
Status: ASSIGNED → NEW
Flags: blocking1.9? → blocking1.9+
There's also double normalization going on in nsXMLContentSink in addition to the expat driver:
http://mxr.mozilla.org/seamonkey/source/content/xml/document/src/nsXMLContentSink.cpp#1492
Yeah, we should remove that too.  At least if expat performs the normalization for us.  Does it?
Attached patch v1 (obsolete) — Splinter Review
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → mozilla1.9alpha6
Attached patch v2Splinter Review
Attachment #267464 - Attachment is obsolete: true
Attachment #267744 - Flags: superreview?(bzbarsky)
Attachment #267744 - Flags: review?(bzbarsky)
Comment on attachment 267744 [details] [diff] [review]
v2

Looks good!
Attachment #267744 - Flags: superreview?(bzbarsky)
Attachment #267744 - Flags: superreview+
Attachment #267744 - Flags: review?(bzbarsky)
Attachment #267744 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.