Setting innerHTML with textarea can insert an extra new line

RESOLVED FIXED

Status

()

Core
HTML: Parser
RESOLVED FIXED
15 years ago
13 years ago

People

(Reporter: Jing Lim, Assigned: mrbkap)

Tracking

({testcase})

Trunk
x86
All
testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 3 obsolete attachments)

(Reporter)

Description

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

15 years ago
Reporter, you are using an old version of Mozilla. 

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

Comment 2

15 years ago
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.
(Reporter)

Comment 3

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

Comment 4

15 years ago
Created attachment 127567 [details]
testcase

Updated

15 years ago
Keywords: testcase

Comment 5

14 years ago
Confirmed on 1.5 using linux
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All

Comment 6

14 years ago
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!
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.
Depends on: 272702

Comment 8

13 years ago
bz: any chance this bug might be fixed for gecko 1.8 ?
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.
(Assignee)

Comment 10

13 years ago
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).
(Assignee)

Comment 11

13 years ago
Created attachment 183854 [details] [diff] [review]
parser changes to make textarea not skipped content

This is the described patch.
(Assignee)

Comment 12

13 years ago
Created attachment 183864 [details] [diff] [review]
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.
Assignee: harishd → mrbkap
Attachment #183854 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(Assignee)

Comment 13

13 years ago
Created attachment 183872 [details] [diff] [review]
patch v2

This addresses sicking's comments over IRC.
Attachment #183864 - Attachment is obsolete: true
Attachment #183872 - Flags: superreview?(bzbarsky)
Attachment #183872 - Flags: review?(bugmail)
Note that the newline problem is really bug 2750. You might want to fix that one
too one day. :-)
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
Attachment #183872 - Flags: review?(bugmail) → review+
(Assignee)

Comment 16

13 years ago
Created attachment 183878 [details] [diff] [review]
patch v2.1

I suck. I forgot to update the other implementations of nsIHTMLContentSink.
These are all straightforward.
Attachment #183878 - Flags: superreview?(bzbarsky)
Attachment #183878 - Flags: review?(bugmail)
(Assignee)

Comment 17

13 years ago
I filed bug 294599 on the string issue.
Attachment #183878 - Flags: review?(bugmail) → review+
(Assignee)

Comment 18

13 years ago
Created attachment 183922 [details] [diff] [review]
updated patch v2
Attachment #183878 - Flags: superreview?(bzbarsky) → superreview+
Attachment #183922 - Flags: superreview+
Attachment #183872 - Flags: superreview?(bzbarsky)
(Assignee)

Comment 19

13 years ago
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.
Attachment #183878 - Flags: approval1.8b2?
(Assignee)

Comment 20

13 years ago
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.
Attachment #183922 - Flags: review+
Attachment #183922 - Flags: approval1.8b2?
(Assignee)

Updated

13 years ago
Attachment #183872 - Attachment is obsolete: true
(Assignee)

Comment 21

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

Asa tells me this won't make 1.8b2.
Attachment #183922 - Flags: approval1.8b2? → approval1.8b3?
(Assignee)

Updated

13 years ago
Attachment #183878 - Flags: approval1.8b2? → approval1.8b3?
Blocks: 293162
Comment on attachment 183878 [details] [diff] [review]
patch v2.1

a=shaver
Attachment #183878 - Flags: approval1.8b3? → approval1.8b3+
Comment on attachment 183922 [details] [diff] [review]
updated patch v2

a=shaver
Attachment #183922 - Flags: approval1.8b3? → approval1.8b3+
(Assignee)

Comment 24

13 years ago
Fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.