99467 - [quirks]options of <a>-tag inside textarea are stripped (compare to source)

Status: RESOLVED FIXED

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

Description

[Jan David Mol]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.3) Gecko/20010801
BuildID:    2001080110

The <a>-tag inside the <textarea> shouldn't be modified by Mozilla.
In mozilla, it shows "<a> ... </a>" when viewing the form, but when
viewing the HTML source, the <a>-tag in fact has a "name" and "target" parameter.

Reproducible: Always
Steps to Reproduce:
1. surf to the page :)
2. scroll down in the textarea
3. compare to source

Actual Results:  mozilla messed up the <a>-tag.

Expected Results:  leave the text between <textarea> and </textarea> alone.

the bug doesn't seem to occur when one just creates a simple form with a
textarea and an <a>-tag inside it.

Comment 1

[Boris Zbarsky [:bzbarsky]]

You _do_ know that you should escape "<" as "&lt;" to prevent just this from
happening, right?

Over to parser; at a guess that's where the issue is....
Seeing this on linux build 2001-09-07-12 as well.

Comment 2

[David Einstein]

Attachment: testcase

Comment 3

[David Einstein]

The problem is that <a> tag gets parsed before the javascript creates the
<textarea tag.  I'm not sure what should happen.
Note:  Teplacing the < with a &lt; causes the textarea tag to show up as text -
the javascript gets the text verbatim, it is the HTML parser that expands the
entities.  I think that this is correct.

Comment 4

[Boris Zbarsky [:bzbarsky]]

David, I meant escaping the "<" in "<a" as "&lt;".  With that change the dynamic
textarea write works too.

Comment 5

[Jan David Mol]

well IMHO the tag should either be converted into a link or left alone,
not changed into <a>. IE4 and NN4.7 show the full <a href=...> tag in
the textarea.

ofcourse, escaping < and > is best :) i couldn't find on www.w3c.org if tags 
inside a textarea are allowed or not in the first place..

Comment 6

[Boris Zbarsky [:bzbarsky]]

> i couldn't find on www.w3c.org if tags inside a textarea are allowed or not in
> the first place

They are not.  If the <textarea> is just a tag in the html, we render it as NS4
and IE do.  It's only with dynamically written textareas, as David pointed out,
that the parser gets confused.

Comment 7

[Christopher Hoess (gone)]

Seems like the < should be escaped...marking quirks.

Comment 8

[harishd]

Not a critical issue. Moving to 0.9.9

Comment 9

[harishd]

Mass moving bugs to 1.1

Comment 10

[Boris Zbarsky [:bzbarsky]]

*** Bug 133518 has been marked as a duplicate of this bug. ***

Comment 11

[Boris Zbarsky [:bzbarsky]]

I was wrong.  Bug 133518 has a testcase with a static textarea that shows this
problem.

Comment 12

[Boris Zbarsky [:bzbarsky]]

*** Bug 156432 has been marked as a duplicate of this bug. ***

Comment 13

[Heikki Toivonen (remove -bugzilla when emailing directly)]

[was 1.1alpha]

Comment 14

[Brian Ryner (not reading)]

blogger.com is a fairly major site that is affected by this bug (see bug
156432). I'd like to suggest that this be reconsidered.  Nominating for next
Netscape release.

Comment 15

[David Hyatt]

I think this needs to be fixed for 1.1.  This bug makes blogger unusable. 
Actually it's even worse than that, since it mangles your blogger template,
which results in data loss.  I have spent hours recovering my blogger templates
because of this bug.

Comment 16

[Marcus Campbell]

For static textareas, Phil Ringnalda has worked out that it is unescaped
|</noscript>|s inside textareas which are causing the bug. (See bug 157800
comment 10)

Comment 17

[Brant Gurganus]

By the definitions on <http://bugzilla.mozilla.org/bug_status.html#severity> and
<http://bugzilla.mozilla.org/enter_bug.cgi?format=guided>, crashing and dataloss
bugs are of critical or possibly higher severity.  Only changing open bugs to
minimize unnecessary spam.  Keywords to trigger this would be crash, topcrash,
topcrash+, zt4newcrash, dataloss.

Comment 18

[Christopher Hoess (gone)]

*** Bug 192968 has been marked as a duplicate of this bug. ***

Comment 19

[Christopher Hoess (gone)]

*** Bug 195272 has been marked as a duplicate of this bug. ***

Comment 20

[harishd]

Did a little bit of investigation ( in fact have a "fix" in hand for this bug )
and found out that the duplicate bugs aren't exactly the same bug. The duplicate
bugs are caused by </noscript> inside textarea while this bug is caused by
document.written <textarea>.

Comment 21

[harishd]

Attachment: Patch v1.0

Making sure to stop preserving content once the target tag's ( that initiated
the content preserving ) matching end tag is encountered. Also, passing this
information to tokenizers that are on the stack. This should fix this bug and
its duplicates.

Note: I haven't run the parser regression test or walked through the top 100
sites yet.

Comment 22

[harishd]

Attachment: Patch v1.1

This patch in addition to fixing this bug and its duplicates also fixes a
completely document.written textarea. That is, it fixes samples like:

<script language="JavaScript">
  document.write('<textarea rows="1", cols="80">')
  document.write('<font style="green"> Mozilla	  </font>');
  document.write('</textarea>');
</script>

Note: Patch v1.0 and v1.1 passed parser regression test.

Comment 23

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 123987 [details] [diff] [review]
Patch v1.1

>Index: htmlparser/public/nsITokenizer.h
>===================================================================
>Index: htmlparser/src/CParserContext.cpp
>===================================================================
>+      // Propagate tokenizer state so that information is presereved

Typo, should be |preserved|.

>Index: htmlparser/src/nsExpatDriver.cpp
>===================================================================
>Index: htmlparser/src/nsHTMLTokenizer.cpp
>===================================================================
>+     nsHTMLTokenizer* rhsTokenizer = NS_STATIC_CAST(nsHTMLTokenizer*, aTokenizer);
>+     mPreserveTarget = rhsTokenizer->mPreserveTarget;

You could get rid of the temporary.

>Index: htmlparser/src/nsHTMLTokenizer.h
>===================================================================
>+  PRInt32            mFlags;

Make it unsigned.

>Index: htmlparser/src/nsParser.cpp
>===================================================================
>+    mParserContext=  oldContext->mPrevContext;

Add space before =.

Basically just syntax review so far.

I'll talk to you in person figure out why this really fixes things. Should also
run visual check on top100 and do page load tests.

Comment 24

[Johnny Stenback (:jst)]

Comment on attachment 123987 [details] [diff] [review]
Patch v1.1

What heikki said, and:

- In nsITokenizer.h:

+  NS_IMETHOD			  CopyStates(nsITokenizer* aTokenizer) = 0;

Don't you want to call this CopyState() and not CopyStates()? Singular sounds
more natural here.

sr=jst

Comment 25

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 123987 [details] [diff] [review]
Patch v1.1

r=heikki after talking this through with harishd.

Land this on trunk first, and if everything goes well and we don't see
regressions in a few days seek permissions to land it on the 1.4 branch.

Comment 26

[harishd]

Attachment: Patch v1.2 [ addresses heikki's and jst's concerns ]

Note: Walked through top 50 sites and they looked ok.

Comment 27

[harishd]

Comment on attachment 124366 [details] [diff] [review]
Patch v1.2 [ addresses heikki's and jst's concerns ]

Carrying forward r/sr=.

Comment 28

[harishd]

Fix landed.

Comment 29

[Asa Dotzler [:asa]]

Comment on attachment 124366 [details] [diff] [review]
Patch v1.2 [ addresses heikki's and jst's concerns ]

a=asa (on behalf of drivers) for checkin to the  1.4 branch.