Bug 60416 - NSCollectionView doesn't support drop
Summary: NSCollectionView doesn't support drop
Status: RESOLVED FIXED
Alias: None
Product: Xamarin.Mac
Classification: Desktop
Component: Library (Xamarin.Mac.dll) ()
Version: 3.8.0 (d15-4)
Hardware: Macintosh Mac OS
: Normal normal
Target Milestone: 15.6
Assignee: Timothy Risi
URL:
: 54882 ()
Depends on:
Blocks:
 
Reported: 2017-10-27 14:57 UTC by Ivan Icin
Modified: 2017-11-13 20:19 UTC (History)
4 users (show)

Tags:
Is this bug a regression?: ---
Last known good build:


Attachments
test case (356.60 KB, application/x-zip-compressed)
2017-10-27 14:57 UTC, Ivan Icin
Details
test case 2 (358.18 KB, application/x-zip-compressed)
2017-10-30 23:07 UTC, Ivan Icin
Details


Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.


Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:
Status:
RESOLVED FIXED

Description Ivan Icin 2017-10-27 14:57:00 UTC
Created attachment 25486 [details]
test case

According to Apple's documentation, for NSCollectionView to support drop one must register dragging types and implement ValidateDrop in the Delegate (and I can confirm that for NSTableView it works like that in Xamarin).

Unfortunately, ValidateDrop never gets called.

Drag works properly.

The test case is in the attachment.
Comment 1 Chris Hamons 2017-10-27 21:50:32 UTC
I ran your sample and immediately get an error from Cocoa when trying to drag.

2017-10-27 16:35:39.033 DropProblem[7353:176263] 'myItemType' is not a valid UTI string.  Cannot set data for an invalid UTI.

Given this, I currently believe there is a programming error in your sample and am not convinced this is a bug in Xamarin.Mac.

Please feel free to post on our XM forums (https://forums.xamarin.com/categories/mac) or do additional digging on your issue.

If you can pin it down to "this API should work like X but it isn't", or even better with a comparison objective-c sample showing the behavior difference, then feel free to reopen.
Comment 2 Ivan Icin 2017-10-27 22:59:22 UTC
Chris, as far as I can tell this wasn't a proper handling of the problem.

As you can see from the above the bug was confirmed by Timothy Risi and he is not a rookie but Xamarin employee that knows at least to test the bug, so at the very least you should have contacted him and check why did he confirm the bug. Not to add that this is fairly important functionality and again after insider confirmation should deserve at least some attention.

I can confirm that this doesn't crash on my Mac. And I am pretty sure that Timothy can confirm too. So we've got the reproducible case in your office and it should be fixible then.

And just to say no it doesn't work with any UTI, including with system UTIs, so if you can't reproduce the problem change it to file, image whatever and try to drag over.
Comment 3 Chris Hamons 2017-10-30 19:10:50 UTC
So a few things:

- When Tim confirmed the bug, he didn't notice the error message from Cocoa. Bug confirmation primarily validates that a bug contains sufficient information to be looked at (a sample that builds, enough information, etc).

- When I did additional digging, I ran into it, which led me to question the example. 

- The fact that the bug was marked confirmed, and then answered might have been confusing.

- Since there is some question on people on the bug, let me clarify. Tim works on the Xamarin.Mac team and was doing triage last week. I'm the lead for Xamarin.Mac, and was doing additional digging once I saw it confirmed. We are both on the same team, and there is no confusion in our actions.

- A sample of code not showing the behavior you desire does not constitute a bug report in itself. As I noted above, Cocoa is reporting an error when you invoke their API, which very likely explains the behavior you are seeing. There are many ways to incorrectly invoke Cocoa which provide the wrong behavior or crash, and a vast majority of them are not Xamarin.Mac bugs.

- Questions related to correctly using a given API are more suited to the forums (https://forums.xamarin.com/categories/mac) or Stack Overflow (https://stackoverflow.com/questions/tagged/xamarin.mac).

- If you have exhausted that, directly ported a working sample from Cocoa, or have a specific API call you believe is working incorrectly (and no error messages from Cocoa when run), please feel free to reopen and we can dig further.

- One last note, the following 3rd party documentation may be useful - https://www.raywenderlich.com/132268/advanced-collection-views-os-x-tutorial
Comment 4 Ivan Icin 2017-10-30 23:07:49 UTC
Created attachment 25510 [details]
test case 2
Comment 5 Ivan Icin 2017-10-30 23:08:21 UTC
I appreciate your willing to help, though I don't fully understand why you have closed the bug when I indicated that I can make another test case if this one is the problem (and as said the methods above perfectly works on NSTableView).

Above is another attachment for the project where only system UTIs are used, so you can try to drag the file over the control and see that ValidateDrop is never called as I said.

Can the bug be reopened now?
Comment 6 Chris Hamons 2017-10-31 14:09:06 UTC
The reason this bug is not reopened is because it is not a bug. There is no flaw in Xamarin.Mac that it is pointing out. You are using the API wrong. Full Stop.

As I linked above, you need to read the API documentation and/or the blog post I linked (https://www.raywenderlich.com/132268/advanced-collection-views-os-x-tutorial).

And this one time, to prove that it is not a bug, I will follow the text in "Drag and Drop in Collection Views" on that link by adding:

	collectionView.SetDraggingSource (NSDragOperation.All, true);
	collectionView.SetDraggingSource (NSDragOperation.All, false);

to ViewController.cs ViewDidLoad() and

		public override bool CanDragItems (NSCollectionView collectionView, NSIndexSet indexes, NSEvent evt)
		{
			return true;
		}

		public override INSPasteboardWriting PasteboardWriterForItem (NSCollectionView collectionView, nuint index)
		{
			return (NSString)"/usr/bin/foo";
		}

to CardsDelegateFlowLayout.cs

and I can drag the item to a terminal/text editor to get the path in question.

Please do not abuse bugzilla for support again. There are a number of resources available, such as the forums, appropriate for such requests.
Comment 7 Ivan Icin 2017-10-31 21:09:52 UTC
You're using the deprecated APIs and that's why it doesn't work: https://stackoverflow.com/questions/43867465/cannot-get-drag-and-drop-to-work-onto-nscollectionview

As soon as I've imported proper APIs it works.

Your code that is a 'proof' is a test case that DRAG works which I have explicitly claimed in my report. You haven't even took a minimal effort to understand that this is about dropping.

To sum up. Drop doesn't work in NSCollectionView (which is kind of major bug as many would expect to reorder items there), and you were not even interested in taking a look. Wow.

You could at least see that I have properly reported drag and drop problem in iOS which is now fixed: https://bugzilla.xamarin.com/show_bug.cgi?id=59944. Possibly even that I made some fairly good apps by now: https://blog.xamarin.com/speech-central-makes-easier-browse-internet-hands-free/?utm_medium=social&utm_campaign=blog&utm_source=twitter&utm_content=speech-central-case-study
Comment 8 Chris Hamons 2017-11-01 12:10:29 UTC
My disagreement with the form of the bug report up to #6 is not a statement of on anyone's programming abilities.

The bug "The non-deprecated ValidateDrop(NSCollectionView, NSDraggingInfo, ref nint, ref NSCollectionViewDropOperation) API does not appear to work", is both actionable and one we will look into.
Comment 9 Timothy Risi 2017-11-01 16:34:19 UTC
To clarify Chris' comment a bit, the example projects you uploaded are both using the old (deprecated, pre-10.11) binding for ValidateDrop.  We do not remove bindings when APIs are deprecated, most of them are still usable.  We simply bind the new APIs as well, and we have the newer NSIndexPath versions of the bindings as well.

Your original sample also had an issue (as Chris pointed out) with using an invalid UTI string, which prevents it from working correctly as well.  The UTI needs to be in the reverse domain format, similar to the app's identifier (in this case, something like 'com.labsii.DropProblem.MyTypeIdentifier').

That being said, after fixing the UTI string and using the NSIndexPath version of ValidateDrop, it still was not being hit.  It turns out the NSIndexPath version is bound incorrectly (it's using 'out' parameters where it needs to be 'ref' parameters).  We are working on fixing that, but in the meantime, as you discovered, you can work around the issue by importing the correct version of ValidateDrop yourself.
Comment 10 Ivan Icin 2017-11-02 10:31:56 UTC
Thanks for clarifications, I do at least partially agree with them.

I was wrong in the first test case, I do use a proper string in my app just when making the test case I haven't checked the documentation whether one needs to use that reverse domain string or it just has to be a unique string (but clearly the string from the original app wouldn't work anyway).

I did try with other ValidateDrop and it didn't work, just currently you have just one documented ValidateDrop here and I used that one in test cases: https://developer.xamarin.com/api/type/MonoMac.AppKit.NSCollectionViewDelegate/

So probably you should also update the documentation.
Comment 11 Timothy Risi 2017-11-02 17:52:02 UTC
After reviewing the situation with our documentation team, I've found the reason for the missing types. Fixing it however will require some infrastructure work and may take a few months.  We're sorry for the inconvenience.
Comment 13 Sebastien Pouliot 2017-11-13 20:19:37 UTC
*** Bug 54882 has been marked as a duplicate of this bug. ***