Skip to content

Fix 13538: Update method Drop of DropTarget.cs #13542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 3, 2025

Conversation

LeafShi1
Copy link
Member

@LeafShi1 LeafShi1 commented May 30, 2025

Fixes #13538

Proposed changes

  • Update method Drop to delete *pdwEffect = DROPEFFECT.DROPEFFECT_NONE before CreateDragEventArgs

Customer Impact

  • The component controls can be moved in design time

Regression?

  • Yes

Risk

  • Minimal

Screenshots

Before

Unable move component controls in DemoConsole application.

cannot.move.mp4

After

Component controls can be moved normally
AfterChange

Test methodology

  • Manually

Test environment(s)

  • .net 10.0.0-preview.6.25278.103
Microsoft Reviewers: Open in CodeFlow

@LeafShi1 LeafShi1 requested a review from a team as a code owner May 30, 2025 09:54
@LeafShi1 LeafShi1 requested a review from Copilot May 30, 2025 09:54
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses issue #13538 by updating the Drop method in DropTarget.cs to properly handle default drag effects and null drag events, restoring design-time component movement.

  • Removed the redundant initial reset of *pdwEffect before creating drag event args
  • Added an else branch to reset *pdwEffect when CreateDragEventArgs returns null
Comments suppressed due to low confidence (2)

src/System.Windows.Forms/System/Windows/Forms/OLE/DropTarget.cs:215

  • The pointer *pdwEffect is used as input to CreateDragEventArgs without being initialized, which could lead to undefined behavior. Consider reintroducing a default assignment (e.g., *pdwEffect = DROPEFFECT.DROPEFFECT_NONE) before this call or passing a local default value instead.
if (CreateDragEventArgs(pDataObj, grfKeyState, pt, *pdwEffect) is { } dragEvent)

src/System.Windows.Forms/System/Windows/Forms/OLE/DropTarget.cs:228

  • No automated tests cover the scenario where CreateDragEventArgs returns null. Add a unit or integration test to verify that *pdwEffect is correctly set to DROPEFFECT_NONE in this branch.
else

@LeafShi1 LeafShi1 requested review from JeremyKuhne and Epica3055 May 30, 2025 09:55
@LeafShi1 LeafShi1 changed the title Fix 13538: Update method Drop of DropTarget.cs Fix 13538: Update method Drop of DropTarget.cs May 30, 2025
Copy link

codecov bot commented May 30, 2025

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 76.59551%. Comparing base (2a0c79b) to head (53fcbb3).
Report is 3 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #13542         +/-   ##
===================================================
+ Coverage   76.59184%   76.59551%   +0.00366%     
===================================================
  Files           3230        3230                 
  Lines         639149      639151          +2     
  Branches       47295       47295                 
===================================================
+ Hits          489536      489561         +25     
+ Misses        146037      146007         -30     
- Partials        3576        3583          +7     
Flag Coverage Δ
Debug 76.59551% <0.00000%> (+0.00366%) ⬆️
integration 18.78908% <0.00000%> (-0.00329%) ⬇️
production 50.99758% <0.00000%> (+0.00836%) ⬆️
test 97.40411% <ø> (ø)
unit 48.40087% <0.00000%> (+0.00802%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@JeremyKuhne JeremyKuhne merged commit e79173e into dotnet:main Jun 3, 2025
9 checks passed
@MelonWang1 MelonWang1 added this to the 10 Preview6 milestone Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable move component controls in DemoConsole application
3 participants