Closed Bug 340800 Opened 18 years ago Closed 18 years ago

[FIX]Line breaks lost in text document's body.innerHTML


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






(Reporter: nevis2us, Assigned: bzbarsky)




(Keywords: fixed1.8.1, regression, Whiteboard: [need regression test])


(3 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20060604 Firefox/
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20060604 Firefox/

Load a multiline plain text file into an iframe
and inspect iframe.document.body.innerHTML property.
No line breaks preserved there.

There's a thread in
new lines missing in innerHTML (FF

Reproducible: Always

Steps to Reproduce:
See above

Actual Results:  
line1 line2 line3

Expected Results:  
As there's currently no standard which covers serialization of text/plain content
within an iframe it makes sense to maintain compatibility with IE6 behaviour
or it may break existing cross-browser code.
Version: unspecified → 1.5.0.x Branch
Assignee: nobody → general
Component: General → DOM: Mozilla Extensions
Product: Firefox → Core
QA Contact: general → ian
Version: 1.5.0.x Branch → Trunk
Confirming with both Firefox release build as well as Firefox trunk nightly on Windows XP.
Ever confirmed: true
OS: Linux → All
Like I said in the newsgroup:

What seems to be going on here is that the plaintext serializer is broken when used in Raw mode -- it loses newlines.  It should just preserve them in Raw mode, imo.

That said, maybe we should switch to just serializing innerHTML as HTML for any case-insensitive document (including text/plain) instead of serializing as the actual content type? 
Flags: blocking1.9a1?
Flags: blocking1.8.1?
There's no indication in the bug that this is a regression.  So while it may be bad, it doesn't seem like it needs to block 1.8.1.
Flags: blocking1.8.1? → blocking1.8.1-
> There's no indication in the bug that this is a regression. 

Sorry about that.  This is in fact a regression from Gecko 1.7; that was mentioned in the first post in the newsgroup thread linked in comment 2.

Specifically, this is a regression from bug 155723.
Blocks: 155723
Flags: blocking1.8.1- → blocking1.8.1?
OK, plussing (although I'm the least convinced of the people in the meeting).
Flags: blocking1.8.1? → blocking1.8.1+
I'm happy to fix this, btw.  But I'd like some feedback on which of the two proposed solutions (or both?) we should take here.
-> boris

Not going to hold FF2 beta1 for this, but we'd be interested in a fix for FF2 beta2.
Assignee: general → bzbarsky
Flags: blocking1.8.1+ → blocking1.8.1-
Whiteboard: [ff2b2]
From testing what IE does it seems like it at least for text/plain content serializes body.innerHTML as HTML as the text ends up being inside a <PRE> tag. Given that, it seems like we should make .innerHTML serialize as HTML as well (for case-insensitive documents).
Attached file Testcase (obsolete) —
Attached file Text file for testcase
Attached file Actual testcase (obsolete) —
Attachment #227354 - Attachment is obsolete: true
Attachment #227356 - Attachment is obsolete: true
Attached patch FixSplinter Review
Attachment #227362 - Flags: superreview?(peterv)
Attachment #227362 - Flags: review?(peterv)
Priority: -- → P3
Summary: Line breaks lost in text document's body.innerHTML → [FIX]Line breaks lost in text document's body.innerHTML
Target Milestone: --- → mozilla1.9alpha
Flags: blocking1.8.1- → blocking1.8.1+
Whiteboard: [ff2b2]
Target Milestone: mozilla1.9alpha → mozilla1.8.1beta2
Target Milestone: mozilla1.8.1beta2 → mozilla1.9alpha
Target Milestone: mozilla1.9alpha → mozilla1.8.1beta2
Attachment #227362 - Flags: superreview?(peterv)
Attachment #227362 - Flags: superreview+
Attachment #227362 - Flags: review?(peterv)
Attachment #227362 - Flags: review+
Fixed on trunk.
Closed: 18 years ago
Flags: blocking1.9a1?
Resolution: --- → FIXED
Attachment #227362 - Flags: approval1.8.1?
Comment on attachment 227362 [details] [diff] [review]

a=darin on behalf of drivers
Attachment #227362 - Flags: approval1.8.1? → approval1.8.1+
Checked in on 1.8 branch.  Need to land a regression test, but we don't really have a setup for this sort of test yet...  davel, we could probably drop a test for this into that test suite you were working on for the about:blank tests, actually.  Is there a bug that covers that yet?
Keywords: fixed1.8.1
Whiteboard: [need regression test]
davel, see comment 18?
Target Milestone: mozilla1.8.1beta2 → mozilla1.9alpha
there is the meta-bug "test-harness" - I'll create a tracking bug for this harness (blocking the master test-harness bug), then block that bug with this one.

As far as this specific test:

1) I think we can use jsunit, since no chrome privs are required by the testcase.

2) I took a quick look at this.  Is the pass condition that iframe.document.body.innerHTML contains newlines and is wrapped by a pre tag?  Or do I need to test the results of assignment ot a textarea and a div, too?
> Is the pass condition that iframe.document.body.innerHTML contains newlines and
> is wrapped by a pre tag? 

Yep.  The assignment is just there to make it human-visible; it's not really part of the test.
Flags: in-testsuite?
added to mochitest

Checking in static/bug340800_iframe.txt;
/cvsroot/mozilla/testing/mochitest/static/bug340800_iframe.txt,v  <--  bug340800_iframe.txt
initial revision: 1.1
RCS file: /cvsroot/mozilla/testing/mochitest/tests/test_bug340800.html,v
Checking in tests/test_bug340800.html;
/cvsroot/mozilla/testing/mochitest/tests/test_bug340800.html,v  <--  test_bug340800.html
initial revision: 1.1
Flags: in-testsuite? → in-testsuite+
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.