Hello noel,
I'd like you to do a code review. Please execute
g4 diff -c 9224996
or point your web browser to
http://mondrian/9224996
to review the following code:
Change 9224996 by [EMAIL PROTECTED] on 2008/12/02 14:48:45 *pending*
Implement desktop.acceptDrag on IE.
This is the IE analogue of Firefox's CL 9207912.
R=noel
[EMAIL PROTECTED]
DELTA=76 (39 added, 21 deleted, 16 changed)
OCL=9224996
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#64 edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc#3
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h#2
edit
...
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop_fake_event_attack.html#5
edit
76 delta lines: 39 added, 21 deleted, 16 changed
Also consider running:
g4 lint -c 9224996
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 9224996 by [EMAIL PROTECTED] on 2008/12/02 14:48:45 *pending*
Implement desktop.acceptDrag on IE.
This is the IE analogue of Firefox's CL 9207912.
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#64 edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc#3
edit
...
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h#2
edit
...
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop_fake_event_attack.html#5
edit
==== //depot/googleclient/gears/opensource/gears/desktop/desktop.cc#64 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/desktop.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/desktop.cc 2008-12-02
14:45:38.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/desktop.cc 2008-12-02
14:02:03.000000000 +1100
@@ -1051,12 +1051,12 @@
if (context->is_exception_set()) return;
std::string16 error;
-#if BROWSER_FF
+#if BROWSER_FF || (BROWSER_IE && !defined(OS_WINCE))
::AcceptDrag(module_environment_.get(),
event_as_js_object.get(),
acceptance,
&error);
-#elif (BROWSER_IE && !defined(OS_WINCE)) || BROWSER_WEBKIT
+#elif BROWSER_WEBKIT
::AcceptDrag(module_environment_.get(),
event_as_js_object.get(),
&error);
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc#3
-
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc
2008-12-02 14:45:38.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.cc
2008-12-02 14:39:56.000000000 +1100
@@ -144,21 +144,9 @@
}
-void AcceptDrag(ModuleEnvironment *module_environment,
- JsObject *event,
- std::string16 *error_out) {
- // TODO(nigeltao): port the following JavaScript to C++.
- // if (isDragEnterOrDragOver) {
- // evt.returnValue = false;
- // evt.cancelBubble = true;
- // }
-}
-
-
-void GetDragData(ModuleEnvironment *module_environment,
- JsObject *event_as_js_object,
- JsObject *data_out,
- std::string16 *error_out) {
+bool GetWindowEvent(ModuleEnvironment *module_environment,
+ CComPtr<IHTMLEventObj> &window_event,
+ CComBSTR &type) {
// We ignore event_as_js_object, since Gears can access the IHTMLWindow2
// and its IHTMLEventObj directly, via COM, and that seems more trustworthy
// than event_as_js_object, which is supplied by (potentially malicious)
@@ -167,28 +155,62 @@
// IHTMLWindow2::get_event (different in that, querying both for their
// IUnknown's gives different pointers).
CComPtr<IHTMLWindow2> window;
- if (FAILED(ActiveXUtils::GetHtmlWindow2(
- module_environment->iunknown_site_, &window))) {
- *error_out = STRING16(L"Could not access the IHtmlWindow2.");
- return;
- }
-
+ return SUCCEEDED(ActiveXUtils::GetHtmlWindow2(
+ module_environment->iunknown_site_, &window)) &&
+ SUCCEEDED(window->get_event(&window_event)) &&
+ SUCCEEDED(window_event->get_type(&type));
+}
+
+
+void AcceptDrag(ModuleEnvironment *module_environment,
+ JsObject *event,
+ bool acceptance,
+ std::string16 *error_out) {
CComPtr<IHTMLEventObj> window_event;
- if (FAILED(window->get_event(&window_event))) {
- *error_out = STRING16(L"Could not access the IHtmlEventObj.");
- return;
- }
-
- if (!window_event) {
+ CComBSTR type;
+ if (!GetWindowEvent(module_environment, window_event, type)) {
// If we get here, then there is no window.event, so we are not in
// the browser's event dispatch.
*error_out = STRING16(L"The drag-and-drop event is invalid.");
return;
}
+ if (type == CComBSTR(L"drop")) {
+ // Do nothing.
+ return;
+
+ } else if (type == CComBSTR(L"dragover") ||
+ type == CComBSTR(L"dragenter") ||
+ type == CComBSTR(L"dragleave")) {
+ CComQIPtr<IHTMLEventObj2> window_event2(window_event);
+ CComPtr<IHTMLDataTransfer> data_transfer;
+ if (!window_event2 ||
+ FAILED(window_event->put_returnValue(CComVariant(false))) ||
+ FAILED(window_event->put_cancelBubble(VARIANT_TRUE)) ||
+ FAILED(window_event2->get_dataTransfer(&data_transfer)) ||
+ FAILED(data_transfer->put_dropEffect(acceptance ? L"copy" : L"none")))
{
+ *error_out = GET_INTERNAL_ERROR_MESSAGE();
+ return;
+ }
+ return;
+
+ } else {
+ *error_out = STRING16(L"The drag-and-drop event is invalid.");
+ return;
+ }
+}
+
+
+void GetDragData(ModuleEnvironment *module_environment,
+ JsObject *event_as_js_object,
+ JsObject *data_out,
+ std::string16 *error_out) {
+ CComPtr<IHTMLEventObj> window_event;
CComBSTR type;
- if (FAILED(window_event->get_type(&type))) {
- *error_out = STRING16(L"Could not access the event type.");
+ if (!GetWindowEvent(module_environment, window_event, type)) {
+ // If we get here, then there is no window.event, so we are not in
+ // the browser's event dispatch.
+ *error_out = STRING16(L"The drag-and-drop event is invalid.");
return;
}
====
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h#2
-
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h
2008-12-02 14:45:38.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ie.h
2008-12-02 14:03:39.000000000 +1100
@@ -39,6 +39,7 @@
void AcceptDrag(ModuleEnvironment *module_environment,
JsObject *event,
+ bool acceptance,
std::string16 *error_out);
void GetDragData(ModuleEnvironment *module_environment,
====
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop_fake_event_attack.html#5
-
c:\devel\srcwingears2/googleclient/gears/opensource/gears/test/manual/drag_and_drop_fake_event_attack.html
====
# action=edit type=text
---
googleclient/gears/opensource/gears/test/manual/drag_and_drop_fake_event_attack.html
2008-12-02 14:45:38.000000000 +1100
+++
googleclient/gears/opensource/gears/test/manual/drag_and_drop_fake_event_attack.html
2008-12-02 14:02:51.000000000 +1100
@@ -78,13 +78,8 @@
}
// TODO(nigeltao): replace all of these browser-specific quirks with
// desktop.acceptDrag(evt, true);
- if (isFirefox) {
+ if (isFirefox || isIE) {
desktop.acceptDrag(evt, true);
- } else if (isIE) {
- if (isDragEnterOrDragOver) {
- evt.returnValue = false;
- evt.cancelBubble = true;
- }
} else if (isSafari) {
evt.preventDefault();
}
