857915 - nsHTMLCopyEncoder should produce text/html for XHTML

Status: NEW

(Core :: DOM: Serializers, defect, P5)

Description

[Drew Willcoxon :adw]

nsHTMLCopyEncoder produces text/html encodings only for HTML documents, but it should do so for XHTML too.  nsHTMLCopyEncoder::SetSelection checks nsIHTMLDocument.IsHTML, and if false (and it is false for XHTML), it treats the document as text and doesn't produce text/html:

> // also consider ourselves in a text widget if we can't find an html document
> nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(mDocument);
> if (!(htmlDoc && mDocument->IsHTML())) {
>   mIsTextWidget = true;

The IsHTML term seems to have been introduced in bug 270145; see bz's bug 270145 comment 5.  (IsCaseSensitive apparently served the purpose that IsHTML serves now.  Bug 487023 made the change.)  I tried removing that term and then copying and pasting from an XHTML document, but when CDATA was present, its closing "]]>" was visible in the resulting paste.  I don't know if there are other problems.

Spun out from bug 723163.  See Henri's comments there about how this is the wrong behavior: bug 723163 comment 13, bug 723163 comment 14, and bug 723163 comment 34.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Of course the problem is that some XHTML documents can't be serialized as text/html....

Comment 2

[Drew Willcoxon :adw]

Are you saying the prospect of nsHTMLCopyEncoder producing text/html for XHTML is doomed from the get-go?  Henri says, "In most normal cases, the text/html serializer can convert XHTML to HTML just fine."  I'm new to how this works -- is there some other class that could (even lossily) translate XHTML to HTML, or could put application/xhtml+xml on the clipboard?

Comment 3

[Boris Zbarsky [:bzbarsky]]

I think what Henri says doesn't contradict what I said...  We can certainly produce text/html, and for most actual uses of XHTML it would work.  In some cases it would produce something quite different from what we started with.  Not necessarily even "lossy", but "different".

Comment 4

[Boris Zbarsky [:bzbarsky]]

I guess the real question is whether we care about those cases.  We may not.

Comment 5

[Mats Palmgren (inactive)]

Ah, bug 703514 comment 2, but apparently it broke some about:memory tests
(bug 703514 comment 13).  I still think we should drop that IsHTML() term :-)

Comment 6

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1472046

Move all DOM bugs that haven't been updated in more than 3 years and has no one currently assigned to P5.

If you have questions, please contact :mdaly.