270145 - Cannot copy/paste a portion of a XHTML page that is in a CDATA section

Status: RESOLVED FIXED

(Core :: DOM: Serializers, defect)

Description

[David Sporn]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.7.5) Gecko/20041108 Firefox/1.0
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; fr-FR; rv:1.7.5) Gecko/20041108 Firefox/1.0

Any part of a page that is inside a CDATA section cannot be copy/pasted.

Reproducible: Always
Steps to Reproduce:
1.Go to a page containing CDATA (e.g.
http://www.sporniket.com/page/blog/david/200411161202)
2.Select all the text
3.Copy the selection and paste it into a text editor.

Actual Results:  
The text that is inside CDATA does not appear in the text editor. If you select
text that is inside CDATA only, the paste operation add nothing in the text editor.

Expected Results:  
The selected text should have been copied into the clipboard so that it could be
pasted.

Workaround : view the source, and copy/paste the CDATA content

Comment 1

[Phil Ringnalda (:philor)]

Attachment: testcase

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm.. I thought this had been fixed in bug 234427 (and the testcase for that bug
no longer seems to be available, so I can't retest).

I traced through this code, though, and it looks like
nsAutoCopyService::NotifySelectionChanged calls nsCopySupport::HTMLCopy no
matter what the content is.  This serializes out the DOM as HTML (with the
<![CDATA[]]> in the string).  Then when we want to get text out,
nsHTMLFormatConverter::ConvertFromHTMLToUnicode creates a parser with
nsPlainTextSerializer as the content sink and reparses the string... but the
HTML parser doesn't actually pass CDATA sections to the content sink as CDATA
but as comments (we have existing bugs on this, I'm pretty sure).  So the text
inside the CDATA gets lost.

I'm not sure what the right thing to do is here, since I'm not sure whether we
event support an XML clipboard flavor... Mike, do you happen to know anothing
about that?  I know you dealt with clipboard once upon a time.

Comment 3

[Sebastian Redl]

The testcase's file ending was changed to xhtml somewhere in the past.
http://stud3.tuwien.ac.at/~e0226430/tutorial/functions.xhtml

It amazes me how multi-layered this bug is - I already fixed two places, and
still it doesn't work.

Comment 4

[Boris Zbarsky [:bzbarsky]]

So here's another interesting thing. nsCopySupport::IsPlainTextContext returns
true if the document is not an nsIHTMLDocument.  Which is true for XML, but
definitely not true for XHTML.  I'm not sure how that helps, since
nsHTMLCopyEncoder::Init ignores aMIMEType, and the HTMLCopy() code creates an
HTMLCopyEncoder unconditionally... but if I view the same data as text/xml, I
can copy the CDATA content.

Comment 5

[Boris Zbarsky [:bzbarsky]]

And finally, the problem is that the code involved just uses the
nsHTMLFormatConverter if bIsHTMLCopy is set.

Sounds like the real problem is that sanely copying XML is simply unsupported. 
The right fix ought to be to disable HTML copy for XHTML documents....

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Refix the "non-HTML documents are all plaintext and text controls" hacks

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 167578 [details] [diff] [review]
Refix the "non-HTML documents are all plaintext and text controls" hacks

jst, would you review?

Comment 8

[Boris Zbarsky [:bzbarsky]]

I've filed bug 272649 on the general issue of implementing copying of XML in a
sane way.

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 167578 [details] [diff] [review]
Refix the "non-HTML documents are all plaintext and text controls" hacks

r+sr=jst, but I'd split those if-then's up on individual lines :)

Comment 10

[Sebastian Redl]

So that means my patch worked for text/xml and application/xml, but not
application/xhtml+xml?

Comment 11

[Boris Zbarsky [:bzbarsky]]

Sebastian, that's correct.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Checked in on the trunk, with jst's requested change.