RFR: 8233955: VM crashes if more than one file are added to ClipboardContent via drag and drop [v2]

Kevin Rushforth kcr at openjdk.org
Tue May 16 23:26:51 UTC 2023


On Wed, 10 May 2023 22:31:53 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Lukasz Kostyra has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
>> 
>>  - GlassViewDelegate: Return to deprecated *PboardType symbols
>>    
>>    New NSPasteboardType* symbols were introduced starting 10.13, and we're
>>    targetting 10.12. This deprecation should probably be resolved once we target
>>    macOS 10.13+.
>>    
>>    Old NS*PboardType symbols do not have an equivalent of NSPasteboardTypeFileURL,
>>    so this branch was removed.
>>  - GlassPasteboard: Correct macOS version information in comments
>>    
>>    New DnD API was introduced starting macOS 10.7, not 10.14
>>  - GlassView: Remove deprecated draggingSourceOperationMask
>>  - DndTest: Update instructions
>>    
>>    On Mac to enable moving you must press Cmd, not Shift.
>>  - Restore logic responsible for Cmd key support
>>  - Merge branch 'master' into dnd_multiple_items-JDK-8233955
>>  - Add DnD Multiple File manual test
>>  - Fix preview position; cleanup code
>>  - Add image previews and finish DnD code
>>    
>>    Fixes issues with tests caused by first commit.
>>    
>>    Removes old code used as placeholder.
>>  - Migrate DnD native code to DraggingSession API
>>    
>>    Previous implementation used dragImage call which is deprecated since macOS 10.14. Additionally,
>>    10.14 introduced a restriction not allowing for more than one drag item in the Pasteboard. This
>>    change fixes crashes caused by old API use when DnD-ing more than one item.
>
> This fixes the problem, but there is one regression in behavior I noticed. To reproduce:
> 
> 1. Run `DndTest` (in the same manual tests dir as the one you added)
> 2. Hold down the CMD key
> 3. Drag the "DRAG ME" label to the "DROP ME" (while holding down CMD)
> 
> BUG: When you release the button, the drag does not complete. The expected behavior is that the drag completes with a transfer mode of "MOVE"

> @kevinrushforth now that #1139 is up, should I revert [fa42b1b](https://github.com/openjdk/jfx/commit/fa42b1b13755dec05d77b3865ed657e954854286)?

I think you could either revert the changes you mention (so that we won't be introducing any additional deprecated constants as part of this fix) or do it in a follow-up issue. Whichever you prefer.

> I could update more constants in the process too to keep DnD code clean of deprecated constants, kind of like #1137 does

That seems best left for a follow-on fix.

Btw, I'll take a look at the updated PR tomorrow.

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

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


More information about the openjfx-dev mailing list