358094 - share more code between cocoa drag service and clipboard

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Josh Aas]

There is a bunch of code that could be shared between the Cocoa drag service and the clipboard implementations (for example, image formatting code). Once some of the more serious drag service regressions and bugs are fixed we should figure out exactly what the drag service can share with the clipboard and do it.

Comment 1

[Colin Barrett [:cbarrett]]

Stan, maybe you want to look at or think about this? I was going to originally but that was when the drag & drop bug were in my queue.

Comment 2

[Stan Shebs]

I have been scowling at some of the duplication...

Comment 3

[Josh Aas]

We were waiting for Neil Deakin's big drag image patch to land before cleaning this up. Go for it! The big thing is the cairo image processing stuff.

Comment 4

[Colin Barrett [:cbarrett]]

Attachment: fix v1.0 - first pass

No functional changes.

Added two public static helper methods to nsClipboard, nsDragService calls them when needed.

I'm not sharing the pasteboard writing code because the wildcard behavior is non-obvious and very dragging specific.

Comment 5

[Josh Aas]

Comment on attachment 267758 [details] [diff] [review]
fix v1.0 - first pass

In nsDragService.h you can remove the following two lines and the space above them as well:

class nsILocalFile;
class nsIDOMDragEvent;

You added a couple spaces in the following code:

-  return NS_OK;
+  return NS_OK;  

It looks like you simply removed the following code and I don't think it should be removed:

-      // The DOM only wants LF, so convert from MacOS line endings to DOM line endings.
-      nsLinebreakHelpers::ConvertPlatformToDOMLinebreaks(flavorStr, (void**)&clipboardDataPtr, (PRInt32*)&dataLength);

Comment 6

[Josh Aas]

Attachment: fix v1.1

This fixes my review comments, adds a little more cleanup, most significantly changing the static function names to be more clear about what they are doing.

Comment 7

[Colin Barrett [:cbarrett]]

Comment on attachment 269850 [details] [diff] [review]
fix v1.1

I agree, these are much better method names. r=me

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 269850 [details] [diff] [review]
fix v1.1

+nsClipboard::GetNativeClipboardData(nsITransferable * aTransferable, PRInt32 aWhichClipboard)

Please, no space between nsITransferable and * (similar comments apply in other places)

Also you have nsITransferable *aTransferable in new code in at least one place, please be consistent and put the * after the type

Comment 9

[Josh Aas]

landed on trunk