Add RCW for IDataObject #7519

kant2002 · 2022-07-31T07:02:26Z

Fixes #7518

Microsoft Reviewers: Open in CodeFlow

Fixes dotnet#7518

kant2002 · 2022-07-31T13:04:25Z

Or better, I should just finish #7087

RussKie · 2022-08-01T01:19:29Z

Thank you. However, the biggest qestion here is how we test this? And ensure we haven't missed anything else...

Or better, I should just finish #7087

We're in RC1 and the runway is too small for any significant changes, and risks are too great

willibrandon · 2022-08-01T05:31:53Z

I can help test this, possibly. It needs to be an end to end test like the drag & drop tests in UIIntegration, but the drag operation needs to start from Explorer. My problem is I don't know how to automate the mouse drag & drop operation from Explorer yet.

If I knew how to get the coordinates of a file located on the screen, I could write the test.

kant2002 · 2022-08-01T06:37:11Z

I thin we need something more complicated then UIIntegration. Maybe some "test running" which will run EXE files and then perform interaction over the OS. Maybe we can automate interaction using https://github.com/Microsoft/WinAppDriver . Probably that way we will test and Accessibility itself albeit indirectly.

kant2002 · 2022-08-01T07:24:33Z

Basically less risky change would be undo these 2 changes in this file https://github.com/dotnet/winforms/pull/6976/files#diff-efdc83ac4513276157401c19a46f9c20f7c2aa84d9b9eb1cd19f37aca86420d5R33
Also we should add tests, so this issue will never appears on our radar.

RussKie · 2022-08-02T02:25:53Z

I can help test this, possibly. It needs to be an end to end test like the drag & drop tests in UIIntegration, but the drag operation needs to start from Explorer. My problem is I don't know how to automate the mouse drag & drop operation from Explorer yet.

It's possible to open an new instance of Explorer and select a file in it, e.g. cmd /c "explorer /select,<full path to file>". More docs are available at https://ss64.com/nt/explorer.html.
The question is how we can find the screen coordinates of that file... ~~WinAppDriver appears to able to do this somehow: https://github.com/microsoft/WinAppDriver/blob/9c561a52ace7b57bb375bc752389731a68dbcc53/Samples/C%23/NotepadTest/ScenarioPopupDialog.cs#L81.~~

Files in Windows Explorer are displayed in a ListView control, so each file/folder is just a listview item. Accessibility tools (e.g., Narrator, etc.) can work it out, so we should be able too.

Perhaps this could help https://docs.microsoft.com/windows/win32/winauto/uiauto-clientportal and https://docs.microsoft.com/windows/win32/api/uiautomationclient/nf-uiautomationclient-iuiautomation-getfocusedelement

Pinging @vladimir-krestov for ideas.

willibrandon · 2022-08-02T06:23:28Z

It's possible to open an new instance of Explorer and select a file in it, e.g. cmd /c "explorer /select,". More docs are available at https://ss64.com/nt/explorer.html.

Thank you.

I'm still trying to answer the question of how we can reliably find the screen coordinates of that file, but for the record, I can reproduce the issue in a UIIntegration test if I hard code the Explorer screen coordinates and drag to the application.

willibrandon · 2022-08-02T06:33:41Z

A manual test that already exists and reproduces this issue is in WinformsControlsTest.DragDrop.

willibrandon · 2022-08-02T06:34:44Z

I will test these changes.

willibrandon · 2022-08-02T07:25:01Z

src/System.Windows.Forms.Primitives/src/Interop/WinFormsComWrappers.DataObjectWrapper.cs

+                    IDataObjectVtbl.STGMEDIUM_Raw mediumRaw;
+                    ((delegate* unmanaged<IntPtr, FORMATETC*, IDataObjectVtbl.STGMEDIUM_Raw*, HRESULT>)(*(*(void***)_wrappedInstance + 3)))
+                        (_wrappedInstance, pFormat, &mediumRaw).ThrowIfFailed();
+                    medium.pUnkForRelease = Marshal.GetObjectForIUnknown(mediumRaw.pUnkForRelease);


What if pUnk is Zero here? I'm seeing an ArgumentNullException in my initial tests.

merriemcgaw · 2022-08-03T00:12:38Z

Basically less risky change would be undo these 2 changes in this file https://github.com/dotnet/winforms/pull/6976/files#diff-efdc83ac4513276157401c19a46f9c20f7c2aa84d9b9eb1cd19f37aca86420d5R33 Also we should add tests, so this issue will never appears on our radar.

@kant2002 the team has chatted and we think it's probably for the best that we do as you suggest here for .NET 7, and we take the completed fix in .NET 8 with the additional testing. Does that work for you?

kant2002 · 2022-08-03T01:31:05Z

Yes. I think so too. I think smallest thing to undo would be just 2 lines but if you think whole PR should be undone I'm fine. We should have stable WinForms a priority

dreddy-work · 2022-08-03T04:18:33Z

Yes. I think so too. I think smallest thing to undo would be just 2 lines but if you think whole PR should be undone I'm fine. We should have stable WinForms a priority

if we do not have any other known scenario regressed and this reported scenario is fixed by undoing those two lines, we should stick to those lines in this and monitor for any new reports. Meanwhile, I will ask our test team to run some drag/drop scenarios to see if there are any pri0 scenarios that need to be addressed?

Olina-Zhang · 2022-08-04T02:03:05Z

@kant2002 tested this PR using 2 sample projects, the issue still repro. Please take a look, thanks.
One project is from https://github.com/c-ohle/RationalNumerics with targeting the latest .Net 7.0

Another is the one we created about drag-dropping image on pictureBox in Winforms from file explorer:
WinFormsApp14.zip

dreddy-work · 2022-08-11T00:03:08Z

@kant2002 , did you get a chance to look latest comments here?

RussKie · 2022-08-11T04:54:33Z

Basically less risky change would be undo these 2 changes in this file #6976 (files)

Would you be able to action this? The window for RC1 is closing fast.

kant2002 · 2022-08-11T05:07:38Z

If only undo, I can do that.

dreddy-work · 2022-08-12T17:21:14Z

@kant2002 , would it be possible to make the requested changes here before coming Monday?

@Olina-Zhang

Fixes dotnet#7518 Eventually I should improve dotnet#7519 but until then, this is safer approach IMO. cc @Olina-Zhang @dreddy-work @RussKie

Fixes #7518 Eventually I should improve #7519 but until then, this is safer approach IMO.

willibrandon · 2022-08-28T19:42:52Z

@RussKie - I now know how to write the automated test, dragging from Explorer, using IUIAutomation::GetFocusedElement and IUIAutomationElement::GetClickablePoint. Your help has been invaluable.

RussKie · 2022-08-31T02:45:20Z

@RussKie Igor Velikorossov FTE - I now know how to write the automated test, dragging from Explorer, using IUIAutomation::GetFocusedElement and IUIAutomationElement::GetClickablePoint. Your help has been invaluable.

You're giving me too much credit. I randomly blurbed something and you did all the work :) And now I'm eager to learn how to do that, you'll have to show us a test.

willibrandon · 2022-08-31T02:51:38Z

You're giving me too much credit. I randomly blurbed something and you did all the work :) And now I'm eager to learn how to do that, you'll have to show us a test.

I mean all of your help so far, you are like a code whisperer.

I'm unsure how to proceed with the test, should I create a separate issue?

RussKie · 2022-08-31T02:57:33Z

I'm unsure how to proceed with the test, should I create a separate issue?

Yes please.

ghost · 2023-09-30T03:01:10Z

The current status of this "draft" PR has persisted for over 180 days, making it highly probable that it is no longer aligned with the latest codebase. Our repository is set up to automatically close draft PRs that have become outdated, and it requests the author to revisit and reopen them if they deem it necessary, thereby bringing them to the team's attention.

Add RCW for IDataObject

f4e209a

Fixes dotnet#7518

kant2002 requested a review from a team as a code owner July 31, 2022 07:02

ghost assigned kant2002 Jul 31, 2022

dreddy-work requested a review from JeremyKuhne August 1, 2022 04:52

willibrandon reviewed Aug 2, 2022

View reviewed changes

RussKie added the waiting-author-feedback The team requires more information from the author label Aug 11, 2022

ghost removed the waiting-author-feedback The team requires more information from the author label Aug 11, 2022

RussKie marked this pull request as draft August 12, 2022 03:42

ghost added the draft draft PR label Aug 12, 2022

kant2002 added a commit to kant2002/winforms that referenced this pull request Aug 13, 2022

Fix drag and drop

9ded6fb

Fixes dotnet#7518 Eventually I should improve dotnet#7519 but until then, this is safer approach IMO. cc @Olina-Zhang @dreddy-work @RussKie

kant2002 mentioned this pull request Aug 13, 2022

Fix drag and drop #7589

Merged

dreddy-work pushed a commit that referenced this pull request Aug 15, 2022

Fix drag and drop (#7589)

6fd4df2

Fixes #7518 Eventually I should improve #7519 but until then, this is safer approach IMO.

ghost closed this Sep 30, 2023

ghost locked as resolved and limited conversation to collaborators Oct 30, 2023

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add RCW for IDataObject #7519

Add RCW for IDataObject #7519

kant2002 commented Jul 31, 2022 •

edited by ghost

Loading

kant2002 commented Jul 31, 2022

RussKie commented Aug 1, 2022

willibrandon commented Aug 1, 2022

kant2002 commented Aug 1, 2022

kant2002 commented Aug 1, 2022

RussKie commented Aug 2, 2022

willibrandon commented Aug 2, 2022

willibrandon commented Aug 2, 2022

willibrandon commented Aug 2, 2022

willibrandon Aug 2, 2022 •

edited

Loading

merriemcgaw commented Aug 3, 2022

kant2002 commented Aug 3, 2022

dreddy-work commented Aug 3, 2022

Olina-Zhang commented Aug 4, 2022

dreddy-work commented Aug 11, 2022

RussKie commented Aug 11, 2022

kant2002 commented Aug 11, 2022

dreddy-work commented Aug 12, 2022

willibrandon commented Aug 28, 2022 •

edited

Loading

RussKie commented Aug 31, 2022

willibrandon commented Aug 31, 2022

RussKie commented Aug 31, 2022

ghost commented Sep 30, 2023

Add RCW for IDataObject #7519

Add RCW for IDataObject #7519

Conversation

kant2002 commented Jul 31, 2022 • edited by ghost Loading

Microsoft Reviewers: Open in CodeFlow

kant2002 commented Jul 31, 2022

RussKie commented Aug 1, 2022

willibrandon commented Aug 1, 2022

kant2002 commented Aug 1, 2022

kant2002 commented Aug 1, 2022

RussKie commented Aug 2, 2022

willibrandon commented Aug 2, 2022

willibrandon commented Aug 2, 2022

willibrandon commented Aug 2, 2022

willibrandon Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

merriemcgaw commented Aug 3, 2022

kant2002 commented Aug 3, 2022

dreddy-work commented Aug 3, 2022

Olina-Zhang commented Aug 4, 2022

dreddy-work commented Aug 11, 2022

RussKie commented Aug 11, 2022

kant2002 commented Aug 11, 2022

dreddy-work commented Aug 12, 2022

willibrandon commented Aug 28, 2022 • edited Loading

RussKie commented Aug 31, 2022

willibrandon commented Aug 31, 2022

RussKie commented Aug 31, 2022

ghost commented Sep 30, 2023

kant2002 commented Jul 31, 2022 •

edited by ghost

Loading

willibrandon Aug 2, 2022 •

edited

Loading

willibrandon commented Aug 28, 2022 •

edited

Loading