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

Status: RESOLVED FIXED

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

Description

[Alexander Kirillov]

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

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 mozilla.dev.tech.dom:
new lines missing in innerHTML (FF 1.5.0.4)


Reproducible: Always

Steps to Reproduce:
See above

Actual Results:  
line1 line2 line3

Expected Results:  
line1
line2
line3

Comment 1

[Alexander Kirillov]

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.

Comment 2

[Jesse Ruderman]

Link to thread in mozilla.dev.tech.dom:
http://groups.google.com/group/mozilla.dev.tech.dom/browse_frm/thread/56e62fbcb175da80/d38d8128a57cff4d

Comment 3

[Martin Honnen]

Confirming with both Firefox 1.5.0.4 release build as well as Firefox trunk nightly on Windows XP.

Comment 4

[Boris Zbarsky [:bzbarsky]]

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? 

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, plussing (although I'm the least convinced of the people in the meeting).

Comment 8

[Boris Zbarsky [:bzbarsky]]

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.

Comment 9

[Darin Fisher]

-> boris

Not going to hold FF2 beta1 for this, but we'd be interested in a fix for FF2 beta2.

Comment 10

[Johnny Stenback (:jst)]

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).

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Text file for testcase

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Actual testcase

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: Actual testcase for real

Comment 15

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 16

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 17

[Darin Fisher]

Comment on attachment 227362 [details] [diff] [review]
Fix

a=darin on behalf of drivers

Comment 18

[Boris Zbarsky [:bzbarsky]]

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?

Comment 19

[Boris Zbarsky [:bzbarsky]]

davel, see comment 18?

Comment 20

[Dave Liebreich [:davel]]

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?

Comment 21

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 22

[Robert Sayre]

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
done
RCS file: /cvsroot/mozilla/testing/mochitest/tests/test_bug340800.html,v
done
Checking in tests/test_bug340800.html;
/cvsroot/mozilla/testing/mochitest/tests/test_bug340800.html,v  <--  test_bug340800.html
initial revision: 1.1
done