Bug 10634 - -webView:dragDestinationActionMaskForDraggingInfo: is ignored
Summary: -webView:dragDestinationActionMaskForDraggingInfo: is ignored
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-08-29 15:26 PDT by Karl Adam
Modified: 2006-08-30 09:43 PDT (History)
1 user (show)

See Also:


Attachments
Fix file drags not natively understood by WebKit (2.06 KB, patch)
2006-08-29 19:33 PDT, Karl Adam
eric: review-
Details | Formatted Diff | Diff
Second swipe at drag patch with nits taken care of (1.28 KB, patch)
2006-08-30 09:26 PDT, Karl Adam
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Karl Adam 2006-08-29 15:26:48 PDT
I've been having problems where some files were accepted as drops on my custom WebView and others were not, after some debugging I've tracked down where it all screws up and am lost on why it does it at all.

In WebHTMLView on Line 5906 the line:
  if (actionMask & WebDragDestinationActionDHTML) ...

CHANGES actionMask from it's original value to 0. This shouldn't be happening and I can't figure out why it's happening at all, but stepping through the code several times has done it every single time.

After the actionMask has been changed to 0, it of course discards the drag operation since it thinks it's uninterested in the drag and as a result my code waiting on the drop is never called, and the green + icon for copy drags never happens. So basically drags that should be working, fail.
Comment 1 Mark Rowe (bdash) 2006-08-29 15:44:12 PDT
For reference, the line mentioned appears at WebKit/WebView/WebHTMLView.m:5706, not 5906.
Comment 2 Karl Adam 2006-08-29 15:59:28 PDT
he way I've been able to reproduce the bug so far is by having a subclass of WebView in my application that is it's own delegate and returns WebDragDestinationAny for -webView:dragDestinationActionMaskForDraggingInfo: And Mark row is correct, I mistyped 5906 when I meant 5706.
Comment 3 Karl Adam 2006-08-29 17:53:22 PDT
After building everything clean and relaunching Xcode, this is not the source of the problem, just a debugging snafu. Instead it seems what initially thought was the problem, is the problem. 

IN WebNSViewExtras -_web_dragOperationForDraggingInfo: method, [[sender draggingPasteboard] _web_bestURL] is called, and _web_bestURL returns nil if it doesn't know how to display the file type, or handle the schema. So when [WebView canShowFile:] returns NO, it jumps out, and returns nil thus causing the original webDrag method to return NSDragOperationNone and ignore the drag entirely.

I'd submit a patch to just remove the check for canShowFile: since a method called web_bestURL: shouldn't be concerned with whether or not the view can show the URL, merely return the most appropriate URL.
Comment 4 Karl Adam 2006-08-29 19:33:42 PDT
Created attachment 10311 [details]
Fix file drags not natively understood by WebKit
Comment 5 Eric Seidel (no email) 2006-08-29 19:56:37 PDT
Comment on attachment 10311 [details]
Fix file drags not natively understood by WebKit

A couple nits.

1.  * ChangeLog doesn't need to be in the changelog.  (That often happens when you run prepare-ChangeLog twice.)

2.  Normally changes require a copyright line.  In this case, since these are < 5 line changes, they probably don't.  But I just thought I should mention it for the future.

3.  Generally WebKit tries to avoid else statements after returns (as they're unnecessary.)    _web_dragOperationForDraggingInfo could be written cleaner by simply returning NSDragOperationCopy inside the if block and returning NSDragOperationNone outside. Execution only hits the outer return if it didn't trip the if.

Otherwise looks good.

I think we should do one more round on this patch to fix the nits.  (Since it's your first patch, it's good to be extra picky.)  But the substance looks fine.
Comment 6 Karl Adam 2006-08-30 09:26:34 PDT
Created attachment 10316 [details]
Second swipe at drag patch with nits taken care of
Comment 7 Timothy Hatcher 2006-08-30 09:34:14 PDT
Comment on attachment 10316 [details]
Second swipe at drag patch with nits taken care of

This is good. I will take the old ChangeLog and use it.
Comment 8 Timothy Hatcher 2006-08-30 09:43:45 PDT
Landed in r16112.