Hello aa,
I'd like you to do a code review. Please execute
g4 diff -c 8668007
or point your web browser to
http://mondrian/8668007
to review the following code:
Change 8668007 by [EMAIL PROTECTED] on 2008/10/21 15:55:19 *pending*
Make the Drag and Drop JS callbacks take an object as an argument,
which is more future-proof - it allows Gears to provide extra
information (such as the window.event) up to the JavaScript via
the same formal API, and also allows for non-file drops (such as
a vCard dropped onto a DIV).
PRESUBMIT=passed
R=aa
[EMAIL PROTECTED]
DELTA=49 (33 added, 0 deleted, 16 changed)
OCL=8668007
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#5 edit
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.cc#3 edit
...
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop.html#1
edit
49 delta lines: 33 added, 0 deleted, 16 changed
Also consider running:
g4 lint -c 8668007
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 8668007 by [EMAIL PROTECTED] on 2008/10/21 15:55:19 *pending*
Make the Drag and Drop JS callbacks take an object as an argument,
which is more future-proof - it allows Gears to provide extra
information
up to the JavaScript via the same API, and also allows for non-file
drops
(such as, for example, a vCard dropped onto a DIV).
Affected files ...
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#5 edit
... //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.cc#3 edit
...
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop.html#1
edit
==== //depot/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc#5 -
/home/nigeltao/srcgears4/googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
2008-10-21 15:23:41.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drop_target_ff.cc
2008-10-21 15:21:47.000000000 +1100
@@ -116,9 +116,9 @@
if (on_drop_.get() && event_type.Equals(kDragDropAsString)) {
std::string16 error;
- scoped_ptr<JsArray> file_objects(
+ scoped_ptr<JsArray> files_array(
module_environment_->js_runner_->NewArray());
- if (!GetDroppedFiles(drag_session.get(), file_objects.get(), &error)) {
+ if (!GetDroppedFiles(drag_session.get(), files_array.get(), &error)) {
return NS_ERROR_FAILURE;
}
@@ -130,9 +130,13 @@
// that, but that doesn't seem to work, whilst StopPropagation() does.
event->StopPropagation();
+ scoped_ptr<JsObject> context_object(
+ module_environment_->js_runner_->NewObject());
+ context_object->SetPropertyArray(STRING16(L"files"), files_array.get());
+
const int argc = 1;
JsParamToSend argv[argc] = {
- { JSPARAM_ARRAY, file_objects.get() }
+ { JSPARAM_OBJECT, context_object.get() }
};
module_environment_->js_runner_->InvokeCallback(
on_drop_.get(), argc, argv, NULL);
@@ -147,7 +151,14 @@
callback = on_drag_leave_.get();
}
if (callback) {
- module_environment_->js_runner_->InvokeCallback(callback, 0, NULL, NULL);
+ scoped_ptr<JsObject> context_object(
+ module_environment_->js_runner_->NewObject());
+ const int argc = 1;
+ JsParamToSend argv[argc] = {
+ { JSPARAM_OBJECT, context_object.get() }
+ };
+ module_environment_->js_runner_->InvokeCallback(
+ callback, argc, argv, NULL);
}
}
return NS_OK;
==== //depot/googleclient/gears/opensource/gears/desktop/drop_target_ie.cc#3 -
/home/nigeltao/srcgears4/googleclient/gears/opensource/gears/desktop/drop_target_ie.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/desktop/drop_target_ie.cc
2008-10-20 12:30:55.000000000 +1100
+++ googleclient/gears/opensource/gears/desktop/drop_target_ie.cc
2008-10-21 15:26:56.000000000 +1100
@@ -175,8 +175,14 @@
if (!in_file_drop) return E_FAIL;
if (on_drag_enter_.get()) {
+ scoped_ptr<JsObject> context_object(
+ module_environment_->js_runner_->NewObject());
+ const int argc = 1;
+ JsParamToSend argv[argc] = {
+ { JSPARAM_OBJECT, context_object.get() }
+ };
module_environment_->js_runner_->InvokeCallback(
- on_drag_enter_.get(), 0, NULL, NULL);
+ on_drag_enter_.get(), argc, argv, NULL);
}
hr = CancelEventBubble(html_event_obj, html_data_transfer);
@@ -193,8 +199,14 @@
if (FAILED(hr)) return hr;
if (on_drag_over_.get()) {
+ scoped_ptr<JsObject> context_object(
+ module_environment_->js_runner_->NewObject());
+ const int argc = 1;
+ JsParamToSend argv[argc] = {
+ { JSPARAM_OBJECT, context_object.get() }
+ };
module_environment_->js_runner_->InvokeCallback(
- on_drag_over_.get(), 0, NULL, NULL);
+ on_drag_over_.get(), argc, argv, NULL);
}
hr = CancelEventBubble(html_event_obj, html_data_transfer);
@@ -211,8 +223,14 @@
if (FAILED(hr)) return hr;
if (on_drag_leave_.get()) {
+ scoped_ptr<JsObject> context_object(
+ module_environment_->js_runner_->NewObject());
+ const int argc = 1;
+ JsParamToSend argv[argc] = {
+ { JSPARAM_OBJECT, context_object.get() }
+ };
module_environment_->js_runner_->InvokeCallback(
- on_drag_leave_.get(), 0, NULL, NULL);
+ on_drag_leave_.get(), argc, argv, NULL);
}
hr = CancelEventBubble(html_event_obj, html_data_transfer);
@@ -254,16 +272,20 @@
::ReleaseStgMedium(&stg_medium);
std::string16 error;
- scoped_ptr<JsArray> file_objects(
+ scoped_ptr<JsArray> files_array(
module_environment_->js_runner_->NewArray());
if (!FileDialog::FilesToJsObjectArray(
- filenames, module_environment_.get(), file_objects.get(), &error))
{
+ filenames, module_environment_.get(), files_array.get(), &error)) {
return E_FAIL;
}
+ scoped_ptr<JsObject> context_object(
+ module_environment_->js_runner_->NewObject());
+ context_object->SetPropertyArray(STRING16(L"files"), files_array.get());
+
const int argc = 1;
JsParamToSend argv[argc] = {
- { JSPARAM_ARRAY, file_objects.get() }
+ { JSPARAM_OBJECT, context_object.get() }
};
module_environment_->js_runner_->InvokeCallback(
on_drop_.get(), argc, argv, NULL);
====
//depot/googleclient/gears/opensource/gears/test/manual/drag_and_drop.html#1 -
/home/nigeltao/srcgears4/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-21 15:23:41.000000000 +1100
+++ googleclient/gears/opensource/gears/test/manual/drag_and_drop.html
2008-10-21 15:24:02.000000000 +1100
@@ -22,34 +22,34 @@
var eventCount = 0;
var desktop = google.gears.factory.create('beta.desktop');
desktop.registerDropTarget(document.getElementById('dropZone'), {
- 'ondragenter': function() {
+ 'ondragenter': function(context) {
dragEnterCount++;
eventCount++;
document.getElementById('dragEnterOutput').innerHTML =
'Got <b>dragenter</b>: ' + dragEnterCount +
' times, most recent sequence number is ' + eventCount;
},
- 'ondragover': function() {
+ 'ondragover': function(context) {
dragOverCount++;
eventCount++;
document.getElementById('dragOverOutput').innerHTML =
'Got <b>dragover</b>: ' + dragOverCount +
' times, most recent sequence number is ' + eventCount;
},
- 'ondragleave': function() {
+ 'ondragleave': function(context) {
dragLeaveCount++;
eventCount++;
document.getElementById('dragLeaveOutput').innerHTML =
'Got <b>dragleave</b>: ' + dragLeaveCount +
' times, most recent sequence number is ' + eventCount;
},
- 'ondrop': function(files) {
+ 'ondrop': function(context) {
dropCount++;
eventCount++;
var s = 'Got <b>drop</b>: ' + dropCount +
' times, most recent sequence number is ' + eventCount + '<hr/>';
- for (i = 0; i < files.length; i++) {
- var file = files[i];
+ for (i = 0; i < context.files.length; i++) {
+ var file = context.files[i];
s += '<b>' + file.name + '</b> has length <b>' +
file.blob.length + '</b>.<br/>';
}
