Hello aa,
I'd like you to do a code review. Please execute
g4 diff -c 8651940
or point your web browser to
http://mondrian/8651940
to review the following code:
Change 8651940 by [EMAIL PROTECTED] on 2008/10/20 13:32:19 *pending*
Make Firefox file Drag and Drop work on Mac. Previously, it only worked
on Windows and Linux.
This change also changes Windows file DnD to follow .lnk link files.
PRESUBMIT=passed
R=aa
[EMAIL PROTECTED]
DELTA=57 (47 added, 4 deleted, 6 changed)
OCL=8651940
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#4 edit
... //depot/googleclient/gears/opensource/gears/tools/config.mk#88 edit
57 delta lines: 47 added, 4 deleted, 6 changed
Also consider running:
g4 lint -c 8651940
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 8651940 by [EMAIL PROTECTED] on 2008/10/20 13:32:19 *pending*
Make Firefox file Drag and Drop work on Mac. Previously, it only worked
on Windows and Linux.
This change also changes Windows file DnD to follow .lnk link files.
OCL=8651940
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#4 edit
... //depot/googleclient/gears/opensource/gears/tools/config.mk#88 edit
==== //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#4 -
/Users/nigeltao/devel/srcmacgears1/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
2008-10-20 13:32:30.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
2008-10-20 14:27:11.000000000 +1100
@@ -34,6 +34,7 @@
#include <gecko_sdk/include/nsIDOMEvent.h>
#include <gecko_sdk/include/nsIDOMEventTarget.h>
#include <gecko_sdk/include/nsIIOService.h>
+#include <gecko_sdk/include/nsILocalFile.h>
#include <gecko_sdk/include/nsISupportsPrimitives.h>
#include <gecko_sdk/include/nsIURI.h>
#include "gears/base/common/leak_counter.h"
@@ -157,6 +158,7 @@
nsIDragSession *drag_session,
JsArray *files_out,
std::string16 *error_out) {
+#if defined(LINUX) && !defined(OS_MACOSX)
// Note to future maintainers: the nsIIOService docs say that it may only be
// used from the main thread. On the other hand, all we're using it for is
// converting a string like "file:///blah" into a nsIFileURL, rather than
@@ -165,6 +167,23 @@
do_GetService("@mozilla.org/network/io-service;1");
if (!io_service) { return false; }
+ // Although Firefox's underlying XPCOM widget library aims to present a
+ // consistent cross-platform interface, there are still significant
+ // differences in drag-and-drop. In particular, different OSes support
+ // different "data flavors". On Windows and on Mac, we can specify the
+ // kFileMime flavor to get an nsIFile object off the clipboard, but on
+ // Linux there is no kFileMime, and we have to specify kURLMime instead
+ // to get the "file:///home/user/foo.txt" string to convert to a filename.
+ // For implementation details, look at Firefox's source code, particularly
+ // nsClipboard.cpp and nsDragService.cpp in widget/src/{gtk2,mac,windows}.
+ // On platforms, such as Windows, where both kURLMime and kFileMime are
+ // supported, then the latter is preferred because it allows us to follow
+ // links (e.g. .lnk files on Windows, and alias files on Mac).
+ const char *flavor = kURLMime;
+#else
+ const char *flavor = kFileMime;
+#endif
+
PRUint32 num_drop_items;
nsresult nr = drag_session->GetNumDropItems(&num_drop_items);
if (NS_FAILED(nr) || num_drop_items <= 0) { return false; }
@@ -173,19 +192,15 @@
for (int i = 0; i < static_cast<int>(num_drop_items); i++) {
nsCOMPtr<nsITransferable> transferable =
do_CreateInstance("@mozilla.org/widget/transferable;1", &nr);
- if (NS_FAILED(nr)) {
- return false;
- }
- transferable->AddDataFlavor("text/x-moz-url");
+ if (NS_FAILED(nr)) { return false; }
+ transferable->AddDataFlavor(flavor);
drag_session->GetData(transferable, i);
nsCOMPtr<nsISupports> data;
- PRUint32 data_length;
- nr = transferable->GetTransferData(
- "text/x-moz-url", getter_AddRefs(data), &data_length);
- if (NS_FAILED(nr)) {
- return false;
- }
+ PRUint32 data_len;
+ nr = transferable->GetTransferData(flavor, getter_AddRefs(data),
&data_len);
+ if (NS_FAILED(nr)) { return false; }
+#if defined(LINUX) && !defined(OS_MACOSX)
nsCOMPtr<nsISupportsString> data_as_xpcom_string = do_QueryInterface(data);
nsString data_as_string;
data_as_xpcom_string->GetData(data_as_string);
@@ -205,7 +220,33 @@
nsString filename;
nr = file->GetPath(filename);
if (NS_FAILED(nr)) { return false; }
-
+#else
+ nsCOMPtr<nsIFile> file(do_QueryInterface(data));
+ if (!file) { return false; }
+ nsString path;
+ nr = file->GetPath(path);
+ if (NS_FAILED(nr)) { return false; }
+
+ nsCOMPtr<nsILocalFile> local_file =
+ do_CreateInstance("@mozilla.org/file/local;1", &nr);
+ if (NS_FAILED(nr)) { return false; }
+ nr = local_file->SetFollowLinks(PR_TRUE);
+ if (NS_FAILED(nr)) { return false; }
+ nr = local_file->InitWithPath(path);
+ if (NS_FAILED(nr)) { return false; }
+
+ nsString filename;
+ PRBool bool_result;
+ if (NS_SUCCEEDED(file->IsSymlink(&bool_result)) && bool_result) {
+ nr = local_file->GetTarget(filename);
+ } else if (NS_SUCCEEDED(file->IsFile(&bool_result)) && bool_result) {
+ nr = local_file->GetPath(filename);
+ } else {
+ nr = NS_ERROR_FAILURE;
+ }
+ if (NS_FAILED(nr)) { return false; }
+#endif
+
filenames.push_back(std::string16(filename.get()));
}
==== //depot/googleclient/gears/opensource/gears/tools/config.mk#88 -
/Users/nigeltao/devel/srcmacgears1/googleclient/gears/opensource/gears/tools/config.mk
====
# action=edit type=text
--- googleclient/gears/opensource/gears/tools/config.mk 2008-10-20
14:27:39.000000000 +1100
+++ googleclient/gears/opensource/gears/tools/config.mk 2008-10-20
14:18:29.000000000 +1100
@@ -355,6 +355,8 @@
# paths here.
CPPFLAGS += -DBROWSER_NPAPI -DBROWSER_WEBKIT -DBROWSER_SAFARI
else
+# TODO(nigeltao): Should we instead have a UNIX flag, rather than calling
+# Mac OS X a "flavor of Linux"??
CPPFLAGS += -DLINUX
endif
