Skip to content
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

[release/9.0] Fix GetFormats and GetDataPresent #12193

Open
wants to merge 1 commit into
base: release/9.0
Choose a base branch
from

Conversation

lonitra
Copy link
Member

@lonitra lonitra commented Sep 21, 2024

Customer Impact

This PR fixes issue #12190 where users receive an incomplete list of formats for the data they are dragging/dropping and receive true when calling GetDataPresent with formats that the DataObject did not have, rendering APIs unusable. This regressed in 9.0 preview 1 while converting our interop code to leverage cswin32. We had accidentally removed a while loop in our GetFormats() method so we only retrieved the first format in the list.
For GetDataPresent, we had updated to check the results of QueryGetData for success, but QueryGetData can return HRESULT.S_FALSE which is considered "success", causing the return of true for formats that the DataObject did not actually have.

Testing

Tested with issue repro scenario to verify this fixes the issue. Adding additional tests to our suite will follow.

Risk

Low. Change involves adding back the while loop we had mistakenly removed and checking the result of QueryGetData explicitly for HRESULT.S_OK as we did in old code.

Microsoft Reviewers: Open in CodeFlow

@lonitra lonitra requested a review from a team as a code owner September 21, 2024 01:46
@lonitra lonitra added 🚫 * NO-MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Sep 21, 2024
Copy link

codecov bot commented Sep 21, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 74.81714%. Comparing base (76005dc) to head (1c19f8f).

Additional details and impacted files
@@                  Coverage Diff                  @@
##           release/9.0      #12193         +/-   ##
=====================================================
+ Coverage     74.81123%   74.81714%   +0.00591%     
=====================================================
  Files             3022        3022                 
  Lines           630138      630135          -3     
  Branches         46785       46784          -1     
=====================================================
+ Hits            471414      471449         +35     
+ Misses          155345      155308         -37     
+ Partials          3379        3378          -1     
Flag Coverage Δ
Debug 74.81714% <20.00000%> (+0.00591%) ⬆️
integration 18.03346% <20.00000%> (+0.00230%) ⬆️
production 47.83408% <20.00000%> (+0.01281%) ⬆️
test 97.00759% <ø> (ø)
unit 44.83753% <20.00000%> (+0.01208%) ⬆️

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚫 * NO-MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) Servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants