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

RESOLVED FIXED in mozilla1.9alpha1

Status

()

defect
P3
normal
RESOLVED FIXED
13 years ago
2 months ago

People

(Reporter: nevis2us, Assigned: bzbarsky)

Tracking

({fixed1.8.1, regression})

Trunk
mozilla1.9alpha1
x86
All
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [need regression test], )

Attachments

(3 attachments, 2 obsolete attachments)

Reporter

Description

13 years ago
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
Reporter

Comment 1

13 years ago
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

Comment 3

13 years ago
Confirming with both Firefox 1.5.0.4 release build as well as Firefox trunk nightly on Windows XP.
Status: UNCONFIRMED → NEW
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.

Comment 9

13 years ago
-> 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).
Posted file Testcase (obsolete) —
Posted file Actual testcase (obsolete) —
Attachment #227354 - Attachment is obsolete: true
Attachment #227356 - Attachment is obsolete: true
Posted 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

Updated

13 years ago
Flags: blocking1.8.1- → blocking1.8.1+
Whiteboard: [ff2b2]
Target Milestone: mozilla1.9alpha → mozilla1.8.1beta2
Target Milestone: mozilla1.8.1beta2 → mozilla1.9alpha

Updated

13 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Flags: blocking1.9a1?
Resolution: --- → FIXED
Attachment #227362 - Flags: approval1.8.1?

Comment 17

13 years ago
Comment on attachment 227362 [details] [diff] [review]
Fix

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?

Comment 22

13 years ago
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
Flags: in-testsuite? → in-testsuite+
Component: DOM: Mozilla Extensions → DOM
Product: Core → Core
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.