273741 - Copy from textarea truncated at first instance of <

Status: RESOLVED FIXED

(Core :: DOM: Serializers, defect)

Description

[Ali Ebrahim]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6) Gecko/20041207
Firefox/1.0+

Copying out of textareas is broken where you're copying a string that contains a
'<' character. It is *sometimes* truncated starting with that character. Best
illustrated by example:

Example 1:
Copying '2><foo bar baz' copies only '2>'.

Example 2:
Copying '<hello world>' copies '' (nothing).

Example 3:
Copying 'contains a '<' character' copies 'contains a '<' character'.
(I don't know why it works here, but not for example 1 and 2)

Comment 1

[Bill Mason]

I think this is bug 273657.

Comment 2

[Ali Ebrahim]

Not sure if its a dupe or not, will leave that call to someone who knows better.

Also, when copying &lt;foo&gt; into a textarea, I get <foo>

Comment 3

[Boris Zbarsky [:bzbarsky]]

Note that this bug is in the right component, with the right ccs, so if someone
feels the urge to dup, please dup bug 273657 here.

Comment 4

[Anne (:annevk)]

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

Comment 5

[Boris Zbarsky [:bzbarsky]]

OK, so this was in fact caused by the checkin for bug 244685.

The problem lies in the following lines in the patch for that bug:

+  // this string may still contain HTML formatting, so we need to remove that too.
+  nsCOMPtr<nsIFormatConverter> htmlConverter =
do_CreateInstance(kHTMLConverterCID);
+  NS_ENSURE_TRUE(htmlConverter, NS_ERROR_FAILURE);
... etc

For the case when bHTMLCopy is false, doing this is WRONG.  It will parse
plaintext (text in text/plain pages, text in form controls, etc) as HTML, which
is totally the wrong thing to do.

Comment 6

[David Gardiner]

Attachment: test-case

textarea element with embedded < and >.

-dave

Comment 7

[David Gardiner]

Attachment: Fix plain text code path

This patch fixes the problem for me... 

thanks for spotting the bug and sorry about the inconvenience.

-dave

Comment 8

[:deanis74]

Comment on attachment 168259 [details] [diff] [review]
Fix plain text code path

>+  nsCOMPtr<nsIFormatConverter> htmlConverter = nsnull;
> 
>   // sometimes we also need the HTML version
>   if (bIsHTMLCopy) {

The |htmlConverter| declaration can be inside the |if|, can it not?

r=me with that

Comment 9

[David Gardiner]

No, as it is used later in the file
(http://lxr.mozilla.org/seamonkey/source/content/base/src/nsCopySupport.cpp#141 )

It is only used within another if (bIsHTMLCopy) block though.

-dave

Comment 10

[[e-mail]]

Comment on attachment 168259 [details] [diff] [review]
Fix plain text code path

sr=me with these nits fixed:

>+  nsCOMPtr<nsIFormatConverter> htmlConverter = nsnull;
nsnull is already the default value, no need to explicitly specify it.

>+    nsCOMPtr<nsISupportsString> plainHTML;
>+    plainHTML = do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID);
No point splitting this, just use nsCOMPtr<nsISupportsString>
plainHTML(do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID));

>+    nsresult plainhtml_rv = plainHTML->SetData(textBuffer);
You don't use this return value; no point creating a variable for it.

>+    nsresult converted_rv = ConvertedData->GetData(plaintextBuffer);
Or this one.

Comment 11

[David Gardiner]

Attachment: Fix plain text code path

Updated as per suggestions, I believe this patch is read to check in now.

-dave

Comment 12

[[e-mail]]

Fix checked in.