[Webkit-unassigned] [Bug 28902] Windows DumpRenderTree should properly set the drop effect for a drag-and-drop operation

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=28902





--- Comment #7 from Adam Roben (aroben) <aroben at apple.com>  2009-09-27 20:03:20 PDT ---
(In reply to comment #6)
> (In reply to comment #5)
> > (From update of attachment 40183 [details] [details])
> > > +            // Reset |didDragEnter| so that another drag started within the same frame works properly.
> > > +            //
> > > +            // Notice, WebView::DragEnter (i.e. webViewDropTarget->DragEnter) initializes the field
> > > +            // |WebView::m_dragData|, which is checked in WebView::DragOver (i.e. webViewDropTarget->DragOver).
> > > +            //
> > > +            // If |didDragEnter| == false, then |WebView::m_dragData| is uninitialized, and by
> > > +            // <http://trac.webkit.org/browser/trunk/WebKit/win/WebView.cpp?rev=48754#L4657>
> > > +            // WebView::DragOver returns the drop effect DROPEFFECT_NONE, which is incorrect.
> > > +            // Hence, we set |didDragEnter| = false on a mouse up to reset the DRT drag-and-drop
> > > +            // machinery.
> > 
> > I don't understand this comment. You say that the behavior of WebView::DragOver
> > "is incorrect". Is there a bug filed about this incorrectness? If so, it would
> > probably be better to reference that bug instead of the Trac link you have now.
> > What is the correct behavior? Maybe this will all become clear when I see the
> > bug.
> 
> Notice, Win DRT emulates the drag-and-drop event loop implemented in DoDragDrop
> <http://msdn.microsoft.com/en-us/library/ms678486%28VS.85%29.aspx>. For any
> drag-and-drop operation that ends with a successful drop on the target, the
> correct call flow in Win DRT is: webViewDropTarget->DragEnter,
> webViewDropTarget->DragOver, webViewDropTarget->Drop (*).
> 
> Suppose there are two such drag-and-drop operations A, B on the same page.
> Tracing through the EventSender code, the call flow for A will be: (*) (and
> will set |didDragEnter| to true). But the call flow for B will be:
> webViewDropTarget->DragOver, webViewDropTarget->Drop (because |didDragEnter| is
> still true!; that is, it is never set to false after A completes). This is
> incorrect. The call flow should be the same for both operations and it should
> be (*).

OK, so it sounds like your comment is explaining a bug that currently exists
but that will be fixed by this patch. I think a much more concise comment would
make this clearer. In fact, I'd recommend just removing everything but the
first sentence of the comment you have in your patch.

> There is no bug filed about this. A better idea is to file a new bug for this
> issue with the fix (setting didDragEnter to false), and remove this change from
> the fix for this bug (#28902). Let me know your thoughts.

I think it's OK to leave this change as part of this patch.

Thanks for clearing this up for me!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.