292607 - Dragging <pre>-formatted text collapses whitespace

Status: VERIFIED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Ulrik Sverdrup]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050328 Camino/0.8+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b2) Gecko/20050328 Camino/0.8+

That cococadev page has an example of text enclosed in <pre> </pre> tags.
Copying and dragging such text is inconsistent; copying preserves whitespace,
while dragging collapses the whitespace to spaces.

The problem depends on <pre> tags, regardless of any other tags like
<blockquote> and <code>.


Similar to Bug 116083; https://bugzilla.mozilla.org/show_bug.cgi?id=116083 but
not the same.

Reproducible: Always

Steps to Reproduce:
1.Select text enclosed in <pre>-tags that contains tabs, linebreaks and multiple
spaces
2. Drag the text to a text editor

Actual Results:  
No whitespace preserved; tabs, linebreaks are converted to spaces.

Expected Results:  
The drag operation should perform identically to Edit->copy, i.e. preserving
whitespace.

Using Mac OS X 10.3.8

Comment 1

[Mike Pinkerton (not reading bugmail)]

not camino specific.

Comment 2

[Thomas K. (:tom)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050419
Firefox/1.0.4

not mac osx specific

Comment 3

[Ulrik Sverdrup]

Attachment: Testcase (valid XHTML 1.1)

A small testcase for the bug.

Note: Selecting and dragging results in collapse of whitespace. _However_: if
you use Select All, and then drag, whitespace is preserved.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Probably because the range passed to the serializer doesn't contain the <pre> start tag, so the serializer doesn't realize that it's dealing with preformatted text?  Chances are, looking at what copy/paste does would help...

Comment 5

[Rich Dougherty]

Attachment: Patch v1

> Chances are, looking at what copy/paste does would help...

Yes it did - thanks! The code in nsCopySupport::HTMLCopy set the nsIDocumentEncoder::OutputPreformatted when generating the text flavor. Changing the drag'n'drop code so also sets this flag seems to fix the problem.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 275290 [details] [diff] [review]
Patch v1

Looks reasonable.  Thanks for doing this!

Comment 7

[[e-mail]]

Comment on attachment 275290 [details] [diff] [review]
Patch v1

With this patch, all drags drag the source of the text, which is incorrect for non-preformatted text. I don't think this is how the copy support creates the plain text copy (except when copying from a textarea) but I don't have a debugger handy right now to check.

Comment 8

[[e-mail]]

Attachment: Copy Copy

I, um, copied, what nsCopySupport::HTMLCopy does.

Comment 9

[Boris Zbarsky [:bzbarsky]]

HTMLCopy uses the nsIDocumentEncoder::OutputPreformatted serialization as input to the format converter.  Shouldn't this do the same?

Comment 10

[[e-mail]]

(In reply to comment #9)
> HTMLCopy uses the nsIDocumentEncoder::OutputPreformatted serialization as input
> to the format converter.  Shouldn't this do the same?

HTMLCopy is weird, because it's called for plaintext copies in certain cases (textareas and plaintext message compose). In these cases the HTMLCopyEncoder automagically turns itself back into a plaintext encoder, so just in case, we pass the OutputPreformatted flag. The flag has no effect on an HTML copy.

Plaintext drag'n'drop doesn't seem to hit this codepath.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 275959 [details] [diff] [review]
Copy Copy

OK, sounds good.

Comment 12

[[e-mail]]

Comment on attachment 275959 [details] [diff] [review]
Copy Copy

This fixes a problem dragging a section of preformatted text (such as most of the text in Bugzilla) where currently all of the text is trimmed onto a single line and loses all of its preformatting.

Comment 13

[Rich Dougherty]

Thanks Neil. After your review, I did go back and realise that using an HTMLConverter was probably required to get things working right. However I was still reading about XPCOM, etc before making the change, to check I got everything right. :-)

Just one comment on your patch... I think that SerializeNodeOrSelection() is now only ever called with an inMode of serializeAsHTML. Do you think it would be worth getting rid of the inMode parameter, the serializationMode enum and removing the unused code paths from the SerializeNodeOrSelection() function? (I'm happy to do this if you'd like.)

Comment 14

[[e-mail]]

(In reply to comment #13)
>I think that SerializeNodeOrSelection() is now only ever called with an inMode
>of serializeAsHTML. Do you think it would be worth getting rid of the inMode
>parameter, the serializationMode enum and removing the unused code paths from
>the SerializeNodeOrSelection() function?
Be my guest :-)

Comment 15

[Rich Dougherty]

Attachment: Patch v3

Thanks Neil. Here's a version of your patch with unused code removed.

Comment 16

[[e-mail]]

Fix checked in.