Hello aa,
I'd like you to do a code review. Please execute
g4 diff -c 8718593
or point your web browser to
http://mondrian/8718593
to review the following code:
Change 8718593 by [EMAIL PROTECTED] on 2008/10/24 18:17:04 *pending*
Add the (mouse) event to the Drag and Drop callback context.
R=aa
[EMAIL PROTECTED]
DELTA=88 (88 added, 0 deleted, 0 changed)
OCL=8718593
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#7 edit
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.h#3 edit
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.cc#6 edit
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.h#4 edit
...
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop.html#4
edit
88 delta lines: 88 added, 0 deleted, 0 changed
Also consider running:
g4 lint -c 8718593
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 8718593 by [EMAIL PROTECTED] on 2008/10/24 18:17:04 *pending*
Add the (mouse) event to the Drag and Drop callback context.
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#7 edit
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.h#3 edit
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.cc#6 edit
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.h#4 edit
...
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop.html#4
edit
==== //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#7 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
2008-10-23 21:03:31.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
2008-10-24 18:17:11.000000000 +1100
@@ -107,6 +107,63 @@
}
+void DropTarget::AddEventToJsObject(JsObject *js_object, nsIDOMEvent *event) {
+ if (!xp_connect_) {
+ nsresult nr = NS_OK;
+ xp_connect_ = do_GetService("@mozilla.org/js/xpc/XPConnect;1", &nr);
+ if (NS_FAILED(nr) || !xp_connect_) {
+ return;
+ }
+ }
+
+ JSContext *js_context = module_environment_->js_runner_->GetContext();
+#if BROWSER_FF2
+ // All we want to do, inside this #if BROWSER_FF2 block, is to call
+ // JSObject *scope = JS_GetScopeChain(js_context);
+ // However, Mozilla bug 390160 says that JS_GetScopeChain sometimes returns
+ // NULL, resulting in a "Nothing active on context" JavaScript error.
+ // This bug was fixed for Gecko 1.9 (i.e. Firefox 3), but we have to
+ // backport the JS_GetScopeChain patch for Firefox2. The patch is at
+ // https://bugzilla.mozilla.org/attachment.cgi?id=274863
+ // and the original bug report is at
+ // https://bugzilla.mozilla.org/show_bug.cgi?id=390160
+ JSObject *global_object = JS_GetGlobalObject(js_context);
+ JSObject *scope = global_object;
+ if (scope) {
+ // The remainder of the backported patch is to call
+ // OBJ_TO_INNER_OBJECT(js_context, scope);
+ // but we don't have access to the OBJ_TO_INNER_OBJECT macro (it is
+ // defined inside mozilla/js/src/jsobj.h), so we just do it manually.
+ JSClass *global_object_class = JS_GetClass(js_context, global_object);
+ if (global_object_class->flags & JSCLASS_IS_EXTENDED) {
+ JSExtendedClass *extended_class = (JSExtendedClass*) global_object_class;
+ if (extended_class->innerObject) {
+ scope = extended_class->innerObject(js_context, scope);
+ }
+ }
+ }
+#else
+ JSObject *scope = JS_GetScopeChain(js_context);
+#endif
+
+ // For more on XPConnect and its various wrapper objects, see
+ // http://wiki.mozilla.org/XPConnect_object_wrapping
+ static const nsIID isupports_iid = NS_GET_IID(nsISupports);
+ nsCOMPtr<nsIXPConnectJSObjectHolder> holder;
+ if (NS_FAILED(xp_connect_->WrapNative(js_context, scope, event,
+ isupports_iid,
+ getter_AddRefs(holder)))) {
+ return;
+ }
+
+ JSObject *object = NULL;
+ if (NS_FAILED(holder->GetJSObject(&object))) {
+ return;
+ }
+ js_object->SetProperty(STRING16(L"event"), OBJECT_TO_JSVAL(object));
+}
+
+
NS_IMETHODIMP DropTarget::HandleEvent(nsIDOMEvent *event) {
nsCOMPtr<nsIDragService> drag_service =
do_GetService("@mozilla.org/widget/dragservice;1");
@@ -139,6 +196,7 @@
scoped_ptr<JsObject> context_object(
module_environment_->js_runner_->NewObject());
context_object->SetPropertyArray(STRING16(L"files"), file_array.get());
+ AddEventToJsObject(context_object.get(), event);
const int argc = 1;
JsParamToSend argv[argc] = {
@@ -159,6 +217,7 @@
if (callback) {
scoped_ptr<JsObject> context_object(
module_environment_->js_runner_->NewObject());
+ AddEventToJsObject(context_object.get(), event);
const int argc = 1;
JsParamToSend argv[argc] = {
{ JSPARAM_OBJECT, context_object.get() }
==== //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.h#3 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drop_target_ff.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drop_target_ff.h
2008-10-23 21:03:31.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drop_target_ff.h
2008-10-24 18:11:18.000000000 +1100
@@ -30,6 +30,7 @@
#else
#include <gecko_internal/nsIDragService.h>
+#include <gecko_internal/nsIXPConnect.h>
#include <gecko_sdk/include/nsIDOMEventListener.h>
#include "gears/base/common/base_class.h"
#include "gears/base/common/js_runner.h"
@@ -76,7 +77,10 @@
static const nsString kDragDropAsString;
nsCOMPtr<nsIDOMEventTarget> event_target_;
+ nsCOMPtr<nsIXPConnect> xp_connect_;
bool unregister_self_has_been_called_;
+
+ void AddEventToJsObject(JsObject *js_object, nsIDOMEvent *event);
DISALLOW_EVIL_CONSTRUCTORS(DropTarget);
};
==== //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.cc#6 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drop_target_ie.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drop_target_ie.cc
2008-10-23 21:03:31.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drop_target_ie.cc
2008-10-24 18:19:03.000000000 +1100
@@ -77,6 +77,15 @@
}
+void DropTarget::AddEventToJsObject(JsObject *js_object) {
+ CComPtr<IHTMLEventObj> html_event_obj;
+ HRESULT hr = html_window_2_->get_event(&html_event_obj);
+ if (FAILED(hr)) { return; }
+ js_object->SetProperty(STRING16(L"event"),
+ CComVariant(static_cast<IDispatch*>(html_event_obj)));
+}
+
+
HRESULT DropTarget::GetHtmlDataTransfer(
CComPtr<IHTMLEventObj> &html_event_obj,
CComPtr<IHTMLDataTransfer> &html_data_transfer)
@@ -136,6 +145,7 @@
if (on_drag_enter_.get()) {
scoped_ptr<JsObject> context_object(
module_environment_->js_runner_->NewObject());
+ AddEventToJsObject(context_object.get());
const int argc = 1;
JsParamToSend argv[argc] = {
{ JSPARAM_OBJECT, context_object.get() }
@@ -160,6 +170,7 @@
if (on_drag_over_.get()) {
scoped_ptr<JsObject> context_object(
module_environment_->js_runner_->NewObject());
+ AddEventToJsObject(context_object.get());
const int argc = 1;
JsParamToSend argv[argc] = {
{ JSPARAM_OBJECT, context_object.get() }
@@ -184,6 +195,7 @@
if (on_drag_leave_.get()) {
scoped_ptr<JsObject> context_object(
module_environment_->js_runner_->NewObject());
+ AddEventToJsObject(context_object.get());
const int argc = 1;
JsParamToSend argv[argc] = {
{ JSPARAM_OBJECT, context_object.get() }
@@ -241,6 +253,7 @@
scoped_ptr<JsObject> context_object(
module_environment_->js_runner_->NewObject());
context_object->SetPropertyArray(STRING16(L"files"), file_array.get());
+ AddEventToJsObject(context_object.get());
const int argc = 1;
JsParamToSend argv[argc] = {
==== //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.h#4 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/desktop/drop_target_ie.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drop_target_ie.h
2008-10-23 21:03:31.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drop_target_ie.h
2008-10-24 18:05:33.000000000 +1100
@@ -117,6 +117,8 @@
DropTarget();
+ void AddEventToJsObject(JsObject *js_object);
+
static _ATL_FUNC_INFO atl_func_info_;
DISALLOW_EVIL_CONSTRUCTORS(DropTarget);
};
====
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop.html#4 -
c:\devel\srcwingears2/googleclient/gears/opensource/gears/test/manual/drag_and_drop.html
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/manual/drag_and_drop.html
2008-10-23 21:03:31.000000000 +1100
+++ googleclient/gears/opensource/gears/test/manual/drag_and_drop.html
2008-10-24 18:08:44.000000000 +1100
@@ -9,6 +9,7 @@
</ol>
<button onclick="toggleEnabledDisabled()">Toggle Enabled / Disabled</button>
<div id="enabledDisabled" style="color:red; font-weight:bold"> </div>
+<div id="eventOutput"> </div>
</div>
<div id="dragEnterOutput"> </div>
<div id="dragOverOutput"> </div>
@@ -27,6 +28,11 @@
var desktop = google.gears.factory.create('beta.desktop');
var dropTargetRegistration = null;
+function updateEventOutput(event) {
+ document.getElementById('eventOutput').innerHTML =
+ 'clientX: ' + event.clientX + ', ' + 'clientY: ' + event.clientY;
+}
+
function toggleEnabledDisabled() {
if (dropTargetRegistration) {
document.getElementById('enabledDisabled').innerHTML =
@@ -39,6 +45,7 @@
'ondragenter': function(context) {
dragEnterCount++;
eventCount++;
+ updateEventOutput(context.event);
document.getElementById('dragEnterOutput').innerHTML =
'Got <b>dragenter</b>: ' + dragEnterCount +
' times, most recent sequence number is ' + eventCount;
@@ -46,6 +53,7 @@
'ondragover': function(context) {
dragOverCount++;
eventCount++;
+ updateEventOutput(context.event);
document.getElementById('dragOverOutput').innerHTML =
'Got <b>dragover</b>: ' + dragOverCount +
' times, most recent sequence number is ' + eventCount;
@@ -53,6 +61,7 @@
'ondragleave': function(context) {
dragLeaveCount++;
eventCount++;
+ updateEventOutput(context.event);
document.getElementById('dragLeaveOutput').innerHTML =
'Got <b>dragleave</b>: ' + dragLeaveCount +
' times, most recent sequence number is ' + eventCount;
@@ -60,6 +69,7 @@
'ondrop': function(context) {
dropCount++;
eventCount++;
+ updateEventOutput(context.event);
var s = 'Got <b>drop</b>: ' + dropCount +
' times, most recent sequence number is ' + eventCount + '<hr/>';
for (i = 0; i < context.files.length; i++) {
