208869 - Setting innerHTML with textarea can insert an extra new line

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Jing Lim]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.2) Gecko/20021216
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.2) Gecko/20021216

The parser seems to behave differently when it's parsing
html versus parsing when javascript sets the innerHTML
of a node.


<textarea>
Line 1
</textarea>

<div id=x>
</div>

<script>
document.getElementById("x").innerHTML = 
  "<textarea>\nShould be Line 1\n<textarea>";
</script>



Reproducible: Always

Steps to Reproduce:
1. Open the HTML shown above.
2.
3.

Actual Results:  
The second textarea shows "Should be Line 1" on the
secondn line.


Expected Results:  
The second textarea should show "Should be first line" on
the first line.

The example works correctly on IE.

Comment 1

[Mikael Parknert]

Reporter, you are using an old version of Mozilla. 

Please verify with a current build (i.e. 1.4rc1)

Comment 2

[Mikael Parknert]

Reporter: Can you reproduce this bug with a recent build of Mozilla (for
example, 1.4rc3)? 

If so, then please comment again with details. 
If not, then please resolve this bug as WORKSFORME. Thanks.

Comment 3

[Jing Lim]

I just tried this on 1.4RC3, the bug is still there.

Comment 4

[Mikael Parknert]

Attachment: testcase

Comment 5

[Morten Nilsen]

Confirmed on 1.5 using linux

Comment 6

[Matthias Krumm]

i have a really similar problem!

here's a link to a test-scenario:
http://www.drumnbass-munich.de/static/test/index.html

for explanation: we push (with js) the content (the textarea and a lot of other
html-code) from a div (implemented in the iframe 'sourceiframe.html'), into a
div in the index.html.
so, if you take a look in this page with netscape/mozilla/firefox/firebird (all
in the newest version), then you will see, that there is a carriage return after
the first few words. and this carriage return is at a position, where there
shouldnt be a carriage return!

Comment 7

[Boris Zbarsky [:bzbarsky]]

The reason for this bug is that the HTML content sink has a nasty hack to remove
a single leading newline from textarea content.  See
http://lxr.mozilla.org/seamonkey/source/content/html/document/src/nsHTMLContentSink.cpp#912

The fragment sink has nothing like this.

Marking dependent on bug 272702 for now; once that's fixed we should revisit this.

Comment 8

[Darin Fisher]

bz: any chance this bug might be fixed for gecko 1.8 ?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Not in a reasonable way, no.  I could probably hack some cases (eg setting
innerHTML on an _ancestor_ of the textarea, which is what that testcase does) or
something, but setting innerHTML on the textarea itself is really not fixable
sanely till we fix bug 272702.

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Actually, textarea is basically ready to not be skipped content in the parser. I
have a patch here (which I'll attach) that makes the HTML parser treat
<textarea> as a regular container. The side effect is that we do even worse on
attachment 127567 [details] because we never remove the first newline (since I'm not
really sure where that should be happening).

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: parser changes to make textarea not skipped content

This is the described patch.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v1

Sicking said, "do it in the parser," so this now strips newlines from textarea
in the parser, passing the testcase in this bug.

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v2

This addresses sicking's comments over IRC.

Comment 14

[Hixie (not reading bugmail)]

Note that the newline problem is really bug 2750. You might want to fix that one
too one day. :-)

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 183872 [details] [diff] [review]
patch v2

>+            if (chop) {
>+              nsAutoString chopped(Substring(start, end));
>+              text->Bind(chopped);
>+            }

Please file a bug on nsScannerSubstring and make an XXX comment reffering to it
here.

r=me

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: patch v2.1

I suck. I forgot to update the other implementations of nsIHTMLContentSink.
These are all straightforward.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

I filed bug 294599 on the string issue.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: updated patch v2

Comment 19

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 183878 [details] [diff] [review]
patch v2.1

This patch, along with attachment 183922 [details] [diff] [review] make setting innerHTML in textareas
not quite so paintful.

Comment 20

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 183922 [details] [diff] [review]
updated patch v2

Carrying over sicking's review, and requesting approval. Along with attachment
183878 [details] [diff] [review], this patch makes using a textarea in innerHTML a lot less painful.

Comment 21

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 183922 [details] [diff] [review]
updated patch v2

Asa tells me this won't make 1.8b2.

Comment 22

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 183878 [details] [diff] [review]
patch v2.1

a=shaver

Comment 23

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 183922 [details] [diff] [review]
updated patch v2

a=shaver

Comment 24

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked in.