140690 - createTextNode turns double newlines into triple newlines (windows-only)

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Comment 1

[Jesse Ruderman]

Attachment: testcase: the three cells should be identical

Comment 2

[Jesse Ruderman]

Even though the summary is "createTextNode turns double newlines into triple
newlines", I think something more complicated is going on.  When I combine this
with bug 140692 ("script.innerHTML doubles all newlines") in attachment 81357 [details],
createTextNode turns some of bug 140692's double newlines back into normal newlines!

Comment 3

[Johnny Stenback (:jst)]

Mass-reassigning bugs.

Comment 4

[Boris Zbarsky [:bzbarsky]]

The three cells in the testcase are in fact identical.  Is this still an issue?

Comment 5

[Jesse Ruderman]

They're not identical for me in Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
rv:1.8a) Gecko/20040508 Firefox/0.8.0+.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Oh, I see.  innerHTML converts DOM-standard newlines (which happen to be Unix
newlines) into platform-specific ones (this is WRONG) and then createTextNode
doesn't convert them back (this is fine).

innerHTML should not be doing newline conversion.

Adding nsIDocumentEncoder::OutputLFLineBreak to the flags when we init the
serializer in nsGenericHTMLElement::GetInnerHTML should fix this, probably. 
Someone with Windows would have to patch and test.

Comment 7

[Martijn Wargers (dead)]

Attachment: Patch based on comment 6

Ok, you mean this? This seems to fix the testcase.

Comment 8

[timeless]

Adding in binary is generally FOO | BAR, not swapping FOO for BAR.

Comment 9

[Johnny Stenback (:jst)]

Yeah, if you'd | in the new flags with the existing flags, I'd sr this patch...

Comment 10

[Martijn Wargers (dead)]

Attachment: Patch2

Ehm, yes, that was stupid of me. I was trying to find out what I broke by
leaving out the original flag (I still don't know), that's why the second,
corrected patch took so long.
Patches from a n00b, but you've probably guessed that already :)

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 150386 [details] [diff] [review]
Patch2

r+sr=jst, thanks for fixing this! (let me know if you need help with checking
this into the tree).

Comment 12

[Martijn Wargers (dead)]

(In reply to comment #11)
> (From update of attachment 150386 [details] [diff] [review])
> r+sr=jst, thanks for fixing this! (let me know if you need help with checking
> this into the tree).
Yes, I need help with that. Thank you!

Comment 13

[Johnny Stenback (:jst)]

Fix checked in, thanks again for the fix!