Fix panic when dragging text onto a window on Windws (#697) (#711)

* Fix panic when dragging text onto a window on Windws (#697)

* Changed `panic` to `debug` (log) when unknow error occurs in `GetData` while processing a drag-drop / hover event. Plus added appropriate cursor effect if hovered item can not be processed.

* Improved code clarity.

* Add documentation to clarify behaviour of `DroppedFile`, `HoveredFile`, and `HoveredFileCancelled`

* Add period at the end of sentences in documentation.

CHANGELOG.md

@@ -7,6 +7,7 @@
 - On Windows, fix issue where resizing or moving window would eat `Awakened` events.
 - On X11, fixed a segfault when using virtual monitors with XRandR.
 - Derive `Ord` and `PartialOrd` for `VirtualKeyCode` enum.
+- On Windows, fix issue where hovering or dropping a non file item would create a panic.
 
 # Version 0.18.0 (2018-11-07)
 

src/events.rs

@@ -37,12 +37,21 @@ pub enum WindowEvent {
     Destroyed,
 
     /// A file has been dropped into the window.
+    /// 
+    /// When the user drops multiple files at once, this event will be emitted for each file
+    /// separately.
     DroppedFile(PathBuf),
 
     /// A file is being hovered over the window.
+    /// 
+    /// When the user hovers multiple files at once, this event will be emitted for each file
+    /// separately.
     HoveredFile(PathBuf),
 
     /// A file was hovered, but has exited the window.
+    /// 
+    /// There will be a single `HoveredFileCancelled` event triggered even if multiple files were
+    /// hovered.
     HoveredFileCancelled,
 
     /// The window received a unicode character.

src/platform/windows/drop_handler.rs

@@ -10,7 +10,7 @@ use winapi::shared::minwindef::{DWORD, MAX_PATH, UINT, ULONG};
 use winapi::shared::windef::{HWND, POINTL};
 use winapi::shared::winerror::S_OK;
 use winapi::um::objidl::IDataObject;
-use winapi::um::oleidl::{IDropTarget, IDropTargetVtbl};
+use winapi::um::oleidl::{DROPEFFECT_COPY, DROPEFFECT_NONE, IDropTarget, IDropTargetVtbl};
 use winapi::um::winnt::HRESULT;
 use winapi::um::{shellapi, unknwnbase};
 
@@ -24,6 +24,8 @@ pub struct FileDropHandlerData {
     pub interface: IDropTarget,
     refcount: AtomicUsize,
     window: HWND,
+    cursor_effect: DWORD,
+    hovered_is_valid: bool, // If the currently hovered item is not valid there must not be any `HoveredFileCancelled` emitted
 }
 
 pub struct FileDropHandler {
@@ -39,6 +41,8 @@ impl FileDropHandler {
             },
             refcount: AtomicUsize::new(1),
             window,
+            cursor_effect: DROPEFFECT_NONE,
+            hovered_is_valid: false,
         });
         FileDropHandler {
             data: Box::into_raw(data),
@@ -77,36 +81,48 @@ impl FileDropHandler {
         pDataObj: *const IDataObject,
         _grfKeyState: DWORD,
         _pt: *const POINTL,
-        _pdwEffect: *mut DWORD,
+        pdwEffect: *mut DWORD,
     ) -> HRESULT {
         use events::WindowEvent::HoveredFile;
         let drop_handler = Self::from_interface(this);
-        Self::iterate_filenames(pDataObj, |filename| {
+        let hdrop = Self::iterate_filenames(pDataObj, |filename| {
             send_event(Event::WindowEvent {
                 window_id: SuperWindowId(WindowId(drop_handler.window)),
                 event: HoveredFile(filename),
             });
         });
+        drop_handler.hovered_is_valid = hdrop.is_some();
+        drop_handler.cursor_effect = if drop_handler.hovered_is_valid {
+            DROPEFFECT_COPY
+        } else {
+            DROPEFFECT_NONE
+        };
+        *pdwEffect = drop_handler.cursor_effect;
 
         S_OK
     }
 
     pub unsafe extern "system" fn DragOver(
-        _this: *mut IDropTarget,
+        this: *mut IDropTarget,
         _grfKeyState: DWORD,
         _pt: *const POINTL,
-        _pdwEffect: *mut DWORD,
+        pdwEffect: *mut DWORD,
     ) -> HRESULT {
+        let drop_handler = Self::from_interface(this);
+        *pdwEffect = drop_handler.cursor_effect;
+
         S_OK
     }
 
     pub unsafe extern "system" fn DragLeave(this: *mut IDropTarget) -> HRESULT {
         use events::WindowEvent::HoveredFileCancelled;
         let drop_handler = Self::from_interface(this);
-        send_event(Event::WindowEvent {
-            window_id: SuperWindowId(WindowId(drop_handler.window)),
-            event: HoveredFileCancelled,
-        });
+        if drop_handler.hovered_is_valid {
+            send_event(Event::WindowEvent {
+                window_id: SuperWindowId(WindowId(drop_handler.window)),
+                event: HoveredFileCancelled,
+            });
+        }
 
         S_OK
     }
@@ -126,7 +142,9 @@ impl FileDropHandler {
                 event: DroppedFile(filename),
             });
         });
-        shellapi::DragFinish(hdrop);
+        if let Some(hdrop) = hdrop {
+            shellapi::DragFinish(hdrop);
+        }
 
         S_OK
     }
@@ -135,12 +153,12 @@ impl FileDropHandler {
         &mut *(this as *mut _)
     }
 
-    unsafe fn iterate_filenames<F>(data_obj: *const IDataObject, callback: F) -> shellapi::HDROP
+    unsafe fn iterate_filenames<F>(data_obj: *const IDataObject, callback: F) -> Option<shellapi::HDROP>
     where
         F: Fn(PathBuf),
     {
         use winapi::ctypes::wchar_t;
-        use winapi::shared::winerror::SUCCEEDED;
+        use winapi::shared::winerror::{SUCCEEDED, DV_E_FORMATETC};
         use winapi::shared::wtypes::{CLIPFORMAT, DVASPECT_CONTENT};
         use winapi::um::objidl::{FORMATETC, TYMED_HGLOBAL};
         use winapi::um::shellapi::DragQueryFileW;
@@ -155,7 +173,8 @@ impl FileDropHandler {
         };
 
         let mut medium = mem::uninitialized();
-        if SUCCEEDED((*data_obj).GetData(&mut drop_format, &mut medium)) {
+        let get_data_result = (*data_obj).GetData(&mut drop_format, &mut medium);
+        if SUCCEEDED(get_data_result) {
             let hglobal = (*medium.u).hGlobal();
             let hdrop = (*hglobal) as shellapi::HDROP;
 
@@ -173,12 +192,16 @@ impl FileDropHandler {
                 }
             }
 
-            return hdrop;
+            return Some(hdrop);
+        } else if get_data_result == DV_E_FORMATETC {
+            // If the dropped item is not a file this error will occur.
+            // In this case it is OK to return without taking further action.
+            debug!("Error occured while processing dropped/hovered item: item is not a file.");
+            return None;
+        } else {
+            debug!("Unexpected error occured while processing dropped/hovered item.");
+            return None;
         }
-
-        // The call to `GetData` must succeed and the file handle must be returned before this
-        // point
-        unreachable!();
     }
 }