On Tue, 23 May 2023 15:24:55 GMT, Lukasz Kostyra <lkost...@openjdk.org> wrote:

>> Crashes started happening due to macOS DnD API change from macOS 10.14 
>> onwards. 10.14 incrodues some [DnD 
>> constrains](https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-10_14#Drag-and-Drop)
>>  which our DnD code happened to trigger on some occasions.
>> 
>> Our code used deprecated `dragImage` API which since 10.14 had new 
>> requirement - each NSPasteboard item had to have a corresponding drag image. 
>> Not meeting this constraint raised an exception, which crashed the app. 
>> Since there was no possibility to add more drag images to `dragImage` API 
>> (it only took one NSImage as parameter) the code had to be rewritten - as 
>> such I upgraded it to new DnD API utilizing NSDraggingSession and related 
>> protocols.
>> 
>> One side-effect of new DnD API is that it now modifies NSPasteboard for us - 
>> previous behavior was more "separated" from user code perspective (write 
>> items to pasteboard -> initiate a drag via `dragImage`). Manually updating 
>> NSPasteboard before calling `beginDraggingSessionWithItems` raised another 
>> exception related to NSPasteboard already having DnD-ed elements inside it. 
>> Some system tests, however, relied on that behavior and writing to 
>> NSPasteboard (`MacPasteboardShim.java` used in some tests creates a 
>> `Clipboard.DND` for test purposes). Since this path is in tests I assumed 
>> this behavior should stay and tried to make it as close to working as 
>> possible. Tests (including those using `MacPasteboardShim`) pass after my 
>> changes.
>> 
>> Additionally, added a new manual test based on `DndTest.java` test which 
>> creates two temporary files and allows for testing faulty behavior.
>
> Lukasz Kostyra has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Release resources to prevent leaks

I updated the code as suggested. Interesting behavior I did not expect, but the 
more you know - thanks for catching that.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1089#issuecomment-1561224875

Reply via email to