Hello noel,

I'd like you to do a code review.  Please execute
        g4 diff -c 9224216

or point your web browser to
        http://mondrian/9224216

to review the following code:

Change 9224216 by [EMAIL PROTECTED] on 2008/12/02 13:25:02 *pending*

        When handling dragenter in Firefox, work around the GTK2 Gecko1.8 bug
        where the DragSession is started *after* the event is dispatched,
        instead of before (the way it is on GTK2 Gecko 1.9, as well as
        Mac and Windows Gecko 1.8 and 1.9).
        
        PRESUBMIT=passed
        R=noel
        [EMAIL PROTECTED]
        DELTA=39  (38 added, 0 deleted, 1 changed)
        OCL=9224216

Affected files ...

... 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#4 
edit

39 delta lines: 38 added, 0 deleted, 1 changed

Also consider running:
        g4 lint -c 9224216

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 9224216 by [EMAIL PROTECTED] on 2008/12/02 13:25:02 *pending*

        When handling dragenter in Firefox, work around the GTK2 Gecko1.8 bug
        where the DragSession is started *after* the event is dispatched,
        instead of before (the way it is on GTK2 Gecko 1.9, as well as
        Mac and Windows Gecko 1.8 and 1.9).

Affected files ...

... 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#4 
edit

==== 
//depot/googleclient/gears/opensource/gears/desktop/drag_and_drop_utils_ff.cc#4 
- 
/home/nigeltao/srcgears4/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-02 13:29:24.000000000 +1100
@@ -205,6 +205,43 @@
   if (event_type.Equals(kDragOverAsString)) {
     return DRAG_AND_DROP_EVENT_DRAGOVER;
   } else if (event_type.Equals(kDragEnterAsString)) {
+#if BROWSER_FF2 && defined(LINUX) && !defined(OS_MACOSX)
+    // The intended Gecko drag and drop model is that (1) the DragService
+    // starts a DragSession (via StartDragSession()), and then (2) a
+    // NS_DRAGDROP_ENTER event is dispatched.
+    // The relevant source files (under mozilla/widget/src) where both
+    // StartDragSession is called and the NS_DRAGDROP_ENTER event is
+    // dispatched are
+    //   Linux: gtk2/nsWindow.cpp
+    //   Mac: cocoa/nsCocoaWindow.mm
+    //   Windows: windows/nsNativeDragTarget.cpp
+    // Windows and Mac follow this model, on Firefox 2 and Firefox 3, and
+    // Linux does this for Firefox 3, but does not for Firefox 2. Instead,
+    // Firefox2/Linux will do these two things in reverse order, i.e. first
+    // dispatch the NS_DRAGDROP_ENTER event, and then StartDragSession. This
+    // is a Gecko bug in nsWindow::OnDragEnter that was fixed in CVS revision
+    // 1.149 of mozilla/widget/src/gtk2/nsWindow.cpp, according to
+    // http://bonsai.mozilla.org/cvslog.cgi?
+    //     file=mozilla/widget/src/gtk2/nsWindow.cpp&rev=1.149
+    //
+    // Gears, however, needs to access the DragSession when handling
+    // drag-enter, to discover file metadata like file count and total size,
+    // and hence we workaround this Firefox2/Linux bug by explicitly calling
+    // StartDragSession here.
+    //
+    // Note that nsBaseDragService::StartDragSession is idempotent, in that
+    // all it does is to set a private member variable (mDoingDrag) to true.
+    // This XPCOM method does return NS_ERROR_FAILURE rather than NS_OK on
+    // the second and subsequent invocations, but that has no practical
+    // significance because StartDragSession is called in only one place
+    // (apart from here in Gears code), namely during nsWindow::OnDragEnter,
+    // and in this case, the nsresult return value is ignored.
+    nsCOMPtr<nsIDragService> drag_service =
+        do_GetService("@mozilla.org/widget/dragservice;1");
+    if (drag_service) {
+      drag_service->StartDragSession();
+    }
+#endif
     return DRAG_AND_DROP_EVENT_DRAGENTER;
   } else if (event_type.Equals(kDragExitAsString)) {
     return DRAG_AND_DROP_EVENT_DRAGLEAVE;
@@ -230,7 +267,8 @@
   if (type == DRAG_AND_DROP_EVENT_DROP) {
     dom_event->StopPropagation();
 
-  } else if (type == DRAG_AND_DROP_EVENT_DRAGOVER) {
+  } else if (type == DRAG_AND_DROP_EVENT_DRAGENTER ||
+             type == DRAG_AND_DROP_EVENT_DRAGOVER) {
     nsCOMPtr<nsIDragService> drag_service =
         do_GetService("@mozilla.org/widget/dragservice;1");
     if (!drag_service) {

Reply via email to