Hello noel,
I'd like you to do a code review. Please execute
g4 diff -c 9210426
or point your web browser to
http://mondrian/9210426
to review the following code:
Change 9210426 by [EMAIL PROTECTED] on 2008/12/01 20:04:40 *pending*
Enable desktop.getDragData for Firefox/Windows and Firefox/OSX,
since the various attacks in test/manual/dnd-fake-event-attack
fail.
PRESUBMIT=passed
R=noel
[EMAIL PROTECTED]
DELTA=22 (10 added, 11 deleted, 1 changed)
OCL=9210426
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#62 edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#4
edit
22 delta lines: 10 added, 11 deleted, 1 changed
Also consider running:
g4 lint -c 9210426
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 9210426 by [EMAIL PROTECTED] on 2008/12/01 20:04:40 *pending*
Enable desktop.getDragData for Firefox/Windows and Firefox/OSX,
since the various attacks in test/manual/dnd-fake-event-attack
fail.
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#62 edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#4
edit
==== //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#62 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/desktop/desktop.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/desktop.cc 2008-12-01
19:55:08.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/desktop.cc 2008-12-01
19:30:24.000000000 +1100
@@ -1088,18 +1088,7 @@
scoped_ptr<JsObject> result(
module_environment_->js_runner_->NewObject());
-#if BROWSER_FF
-#if defined(WIN32) || defined(OS_MACOSX)
- // TODO(nigeltao): test that the implementation is sound for Firefox/Windows
- // and Firefox/Mac.
- error = STRING16(L"getDragData is not supported for this platform.");
-#else
- ::GetDragData(module_environment_.get(),
- event_as_js_object.get(),
- result.get(),
- &error);
-#endif
-#elif (BROWSER_IE && !defined(OS_WINCE)) || BROWSER_WEBKIT
+#if BROWSER_FF || (BROWSER_IE && !defined(OS_WINCE)) || BROWSER_WEBKIT
::GetDragData(module_environment_.get(),
event_as_js_object.get(),
result.get(),
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#4
-
/home/nigeltao/srcgears2/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
2008-12-01 19:55:08.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc
2008-12-01 19:56:05.000000000 +1100
@@ -79,6 +79,7 @@
static bool PointerIsOnTheStack(void *candidate,
void *pointer1,
void *pointer2) {
+#if defined(LINUX) && !defined(OS_MACOSX)
int something_on_the_stack;
if (!pointer1) {
return PointerIsOnTheStack(candidate, &something_on_the_stack, NULL);
@@ -91,6 +92,15 @@
size_t i2 = reinterpret_cast<size_t>(pointer2);
// Check that the sequence (i0, i1, i2) is strictly monotonic.
return (i0 > i1 && i1 > i2) || (i0 < i1 && i1 < i2);
+#else
+ // TODO(nigeltao): the above works for GCC (i.e. Firefox/Linux and
+ // Firefox/OSX), but does not for Visual C++ (i.e. Firefox/Windows).
+ // The TODO is to implement this. It isn't a deal-breaker that this doesn't
+ // work on Windows, since the if (!ns_mouse_event->widget) check during
+ // GetDragAndDropEventType should be sufficient, but still, it would be
+ // nice to have, as a second or third line of defence.
+ return true;
+#endif
}
