On Wed, 17 May 2023 08:50:23 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 two additional > commits since the last revision: > > - GlassViewDelegate: Address review issues > > * Removed draggingSource pointer > * Removed some leftover comments > * Set delegate to nil > - Revert "GlassViewDelegate: Return to deprecated *PboardType symbols" > > This reverts commit fa42b1b13755dec05d77b3865ed657e954854286. > > fa42b1b13 is not needed anymore - JFX will target macOS 11 soon. We spoke about it offline, I was wondering if Apple APIs retain the resources and it turns out they seem to - I added `release`-s as mentioned and everything seems to work fine. I also added a `release` section in GlassPasteboard.m to clean up allocated by me `NSDraggingItem`-s, which after flushing the request and starting the drag should not be needed by us anymore. Tests all pass as they did, did not notice any regressions in both manual and automatic tests. ------------- PR Comment: https://git.openjdk.org/jfx/pull/1089#issuecomment-1558953830
