Verify that the drop source target window is in-process #10319

willibrandon · 2023-11-16T03:27:11Z

Fixes #8903

Proposed changes

Fix the tracking of the last drop target window in DragLeaveTarget.
Notify the drag-image manager during DragLeaveTarget that the cursor has left the target window only if it is in-process.
Check if the target window is in-process and if not, prevent the drag image from being updated during GiveFeeback.
Add and check for additional drag and drop data formats used to detect when the data object is in a drag loop.

Customer Impact

Fixes a drag image flicker and artefact that occurs when dragging and dropping cross-process.

Regression?

No.

Risk

Screenshots

Before

Drag-image-cross-process-before.mp4

After

Drag-image-cross-process-after.mp4

Test methodology

Manual

Test environment(s)

9.0.100-alpha.1.23511.2

Microsoft Reviewers: Open in CodeFlow

* Fix the tracking of the last drop target window in DragLeaveTarget. * Exclude the Effect property from the check for if the drag image has been updated. * Prevent the drag image from being updated during GiveFeedback when the image is updated from another process. * Only notify the drag-image manager that the cursor has left the drop target if the window is in-process.

willibrandon · 2023-11-16T03:47:16Z

@weltkante - Thoughts?

…rmats

JeremyKuhne

Added a few minor comments.

JeremyKuhne · 2023-11-16T18:41:58Z

src/System.Windows.Forms/src/System/Windows/Forms/DragDropHelper.cs

+    internal const string CF_DRAGSOURCEHELPERFLAGS = "DragSourceHelperFlags";
+    internal const string CF_DRAGWINDOW = "DragWindow";
    internal const string CF_DROPDESCRIPTION = "DropDescription";
    internal const string CF_INSHELLDRAGLOOP = "InShellDragLoop";


This one can come from CsWin32 with CFSTR_INDRAGLOOP.

That is cool. First time I've seen that.

JeremyKuhne · 2023-11-16T18:43:10Z

src/System.Windows.Forms/src/System/Windows/Forms/DragDropHelper.cs

    internal const string CF_DRAGIMAGEBITS = "DragImageBits";
+    internal const string CF_DRAGSOURCEHELPERFLAGS = "DragSourceHelperFlags";
+    internal const string CF_DRAGWINDOW = "DragWindow";
    internal const string CF_DROPDESCRIPTION = "DropDescription";


This can be replaced with CFSTR_DROPDESCRIPTION using CsWin32.

JeremyKuhne · 2023-11-16T18:55:43Z

src/System.Windows.Forms/src/System/Windows/Forms/DragDropHelper.cs

 /// </summary>
 internal static class DragDropHelper
 {
    private const int DSH_ALLOWDROPDESCRIPTIONTEXT = 0x0001;


This can come from DSH_FLAGS in CsWin32.

JeremyKuhne · 2023-11-16T18:59:36Z

src/System.Windows.Forms/src/System/Windows/Forms/DragDropHelper.cs

 internal static class DragDropHelper
 {
    private const int DSH_ALLOWDROPDESCRIPTIONTEXT = 0x0001;
+    internal const string CF_COMPUTEDIMAGE = "ComputedImage";


It would be nice to document where these formats come from in the comments here.

I believe a couple of these formats are unnecessary, but I'll document what I know about the ones that are needed.

JeremyKuhne · 2023-11-16T19:11:03Z

src/System.Windows.Forms/src/System/Windows/Forms/DropSource.cs

    }
+
+    private bool IsDropTargetWindowInCurrentThread() => !_lastHwndTarget.IsNull
+        && PInvoke.GetWindowThreadProcessId(_lastHwndTarget, out _) == PInvoke.GetCurrentThreadId();


Maybe we should stash the window's thread to avoid the repeated call? It would also be slightly faster if you also use the unsafe overload and pass null for the process id.

Good idea. Thank you for your help.

weltkante · 2023-11-16T20:10:56Z

@weltkante - Thoughts?

from just looking at the changes it looks good to me, can't see any problems with it. won't have time to tests it myself at the moment.

lonitra

mainly some small stylistic comments, otherwise LGTM!

src/System.Windows.Forms/src/System/Windows/Forms/DropSource.cs

src/System.Windows.Forms/src/System/Windows/Forms/DragDropHelper.cs

src/System.Windows.Forms/src/System/Windows/Forms/GiveFeedbackEventArgs.cs

lonitra

🚀

Feedback addressed

ghost assigned willibrandon Nov 16, 2023

ghost added the draft draft PR label Nov 16, 2023

willibrandon added 2 commits November 15, 2023 21:10

Fix IDataObject NotImplementedExceptions for drag and drop private fo…

7d441a4

…rmats

Fix InDragLoopFormat test

ad93acd

willibrandon marked this pull request as ready for review November 16, 2023 07:35

willibrandon requested a review from a team as a code owner November 16, 2023 07:35

ghost removed the draft draft PR label Nov 16, 2023

JeremyKuhne previously requested changes Nov 16, 2023

View reviewed changes

ghost added the waiting-author-feedback The team requires more information from the author label Nov 16, 2023

ghost removed the waiting-author-feedback The team requires more information from the author label Nov 17, 2023

Use CsWin32, document formats, and optimize the thread id check

230409e

willibrandon requested a review from JeremyKuhne November 17, 2023 07:28

lonitra reviewed Nov 29, 2023

View reviewed changes

lonitra added the waiting-author-feedback The team requires more information from the author label Nov 29, 2023

ghost removed the waiting-author-feedback The team requires more information from the author label Dec 1, 2023

willibrandon added 2 commits November 30, 2023 20:08

Stylistic improvements and revert the GiveFeedbackEventArgs changes

7e210cb

Merge branch 'main' into fix-cross-process-drag-image-artefact

bcbbac4

willibrandon requested a review from lonitra December 1, 2023 04:45

lonitra approved these changes Dec 1, 2023

View reviewed changes

lonitra added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Dec 1, 2023

lonitra merged commit 858bddb into dotnet:main Dec 2, 2023

ghost added this to the 9.0 Preview1 milestone Dec 2, 2023

ghost removed the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Dec 2, 2023

willibrandon deleted the fix-cross-process-drag-image-artefact branch December 2, 2023 21:35

willibrandon mentioned this pull request Dec 17, 2023

Add support for drag images and drop descriptions gitextensions/gitextensions#11423

Closed

github-actions bot locked and limited conversation to collaborators Jan 2, 2024

Verify that the drop source target window is in-process #10319

Verify that the drop source target window is in-process #10319

Uh oh!

Conversation

willibrandon commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Customer Impact

Regression?

Risk

Screenshots

Before

After

Test methodology

Test environment(s)

Microsoft Reviewers: Open in CodeFlow

Uh oh!

willibrandon commented Nov 16, 2023

Uh oh!

JeremyKuhne left a comment

Choose a reason for hiding this comment

Uh oh!

JeremyKuhne Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

willibrandon Nov 17, 2023

Choose a reason for hiding this comment

Uh oh!

JeremyKuhne Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

JeremyKuhne Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

JeremyKuhne Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

willibrandon Nov 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JeremyKuhne Nov 16, 2023

Choose a reason for hiding this comment

Uh oh!

willibrandon Nov 17, 2023

Choose a reason for hiding this comment

Uh oh!

weltkante commented Nov 16, 2023

Uh oh!

lonitra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lonitra left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

willibrandon commented Nov 16, 2023 •

edited

Loading

willibrandon Nov 17, 2023 •

edited

Loading