Hello noel,
I'd like you to do a code review. Please execute
g4 diff -c 12384274
or point your web browser to
http://mondrian/12384274
to review the following code:
Change 12384274 by nigel...@nigeltao-srcwingears2 on 2009/08/19 18:25:32
*pending*
For Firefox/Windows drag-and-drop, revert to not using the
DropTargetInterceptor, since we cannot reliably intercept the right
HWND. For example, if the gears_init.js script is invoked from the
web page's <head>, rather than its <body>, then the right HWND to
intercept doesn't even exist yet.
R=noel
[email protected]
DELTA=35 (32 added, 0 deleted, 3 changed)
OCL=12384274
Affected files ...
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h#9
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#24
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_win32.cc#5
edit
35 delta lines: 32 added, 0 deleted, 3 changed
Also consider running:
g4 lint -c 12384274
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 12384274 by nigel...@nigeltao-srcwingears2 on 2009/08/19 18:25:32
*pending*
For Firefox/Windows drag-and-drop, revert to not using the
DropTargetInterceptor, since we cannot reliably intercept the right
HWND. For example, if the gears_init.js script is invoked from the
web page's <head>, rather than its <body>, then the right HWND to
intercept doesn't even exist yet.
Affected files ...
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h#9
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#24
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_win32.cc#5
edit
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h#9
-
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h
2009-08-19 17:40:23.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_common.h
2009-08-19 18:02:04.000000000 +1000
@@ -33,6 +33,33 @@
#endif
#if GEARS_DRAG_AND_DROP_API_IS_SUPPORTED_FOR_THIS_PLATFORM
+
+
+#if defined(WIN32)
+#if BROWSER_IE
+#define GEARS_DRAG_AND_DROP_USES_INTERCEPTOR 1
+#elif BROWSER_FF
+// Whilst GEARS_DRAG_AND_DROP_USES_INTERCEPTOR mostly works for Firefox/Win32,
+// some bugs remain. In particular, if the gears_init.js script is invoked in
+// the HTML HEAD, instead of the HTML BODY, then during the Gears Factory
+// construction, the HWND that we want to intercept doesn't actually exist yet
+// (and is probably generated when the <body> tag is parsed).
+// The interceptor instead latches on to an incorrect HWND (since there are
+// multiple matches to EnumChildWindows, and we're simply using last-one-wins,
+// and we get weird behavior, such as the first drag-and-drop of files onto a
+// web page works, but subsequent ones only yield the first DnD's files.
+//
+// Thus, on Firefox/Windows, we do not use a DropTargetInterceptor. Ideally,
+// we would want to use one, since that would give us cursor control, but
+// until we get the HWND selection 100% correct, especially in the script-in-
+// head-tag case, then we don't use one. For now, of our two imperfect options,
+// we take (1) the inability to call setDragCursor with anything other than
+// 'none', instead of (2) returning the wrong fileset from getDragData.
+//
+//#define GEARS_DRAG_AND_DROP_USES_INTERCEPTOR 1
+#endif
+#endif
+
#include <set>
#include <vector>
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#24
-
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc
2009-08-19 17:19:36.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc
2009-08-19 18:22:48.000000000 +1000
@@ -307,7 +307,7 @@
return;
}
-#ifdef WIN32
+#if GEARS_DRAG_AND_DROP_USES_INTERCEPTOR
if (module_environment->drop_target_interceptor_) {
module_environment->drop_target_interceptor_->SetDragCursor(cursor_type);
}
@@ -582,7 +582,7 @@
error_out);
#else
-#if defined(WIN32)
+#if GEARS_DRAG_AND_DROP_USES_INTERCEPTOR
if (!module_environment->drop_target_interceptor_) {
return false;
}
@@ -664,7 +664,7 @@
filenames.push_back(std::string16(filename.get()));
}
-#if defined(WIN32)
+#if GEARS_DRAG_AND_DROP_USES_INTERCEPTOR
FileDragAndDropMetaData &file_drag_and_drop_meta_data =
module_environment->drop_target_interceptor_->
GetFileDragAndDropMetaData();
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_win32.cc#5
-
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_win32.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_win32.cc
2009-08-19 17:19:36.000000000 +1000
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_win32.cc
2009-08-19 18:22:14.000000000 +1000
@@ -260,6 +260,7 @@
DropTargetInterceptor *DropTargetInterceptor::Intercept(
ModuleEnvironment *module_environment) {
+#if GEARS_DRAG_AND_DROP_USES_INTERCEPTOR
HWND hwnd_to_intercept = 0;
EnumChildWindowsProcContext context;
context.result_ptr = &hwnd_to_intercept;
@@ -303,6 +304,10 @@
}
}
return interceptor;
+
+#else
+ return NULL;
+#endif // GEARS_DRAG_AND_DROP_USES_INTERCEPTOR
}
