Skip to content

[release/9.0] Only Unwrap If Given IDataObject#12745

Merged
lonitra merged 1 commit intodotnet:release/9.0from
lonitra:backport12738
Jan 14, 2025
Merged

[release/9.0] Only Unwrap If Given IDataObject#12745
lonitra merged 1 commit intodotnet:release/9.0from
lonitra:backport12738

Conversation

@lonitra
Copy link
Member

@lonitra lonitra commented Jan 10, 2025

Backport of #12738

Customer Impact

When users set an object that is not an IDataObject onto our Clipboard using the SetDataObject API, calls to our clipboard APIs with autoConvert = false (either directly or indirectly) will no longer recognize equivalent clipboard formats as it had done in .NET 8, producing unintended behavior change.

This regressed because in .NET 9, we had switched to leveraging cswin32 and ComWrappers for our OLE interop. During this we discovered that switching from built-in COM to ComWrappers took away the ability to automatically cast native pointers back to their original managed object when round tripping through native code. In order to remedy this, we updated to wrap the data when setting it onto the clipboard and unwrap manually on clipboard retrieval before returning it in order to get the original data.

While doing this, however, we had missed the fact that the native IDataObject interface doesn't have the concept of explicit autoConvert. Meaning that when calling our Clipboard APIs with autoConvert = false, it will effectively be ignored and produce same results as autoConvert = true, recognizing equivalent clipboard formats if we're calling to a native IDataObject interface. In .NET 9, since we always unwrap the object retrieved from the clipboard to get our completely managed DataObject, we lost this behavior for types that don't already derive from managed IDataObject.

Testing

Run on regression test suite and added unit tests covering scenario.

Risk

Low. The fix is that when we wrap types that don't already derive from the managed IDataObject, we won't unwrap to the wrapping DataObject we created so that calls go through the native IDataObject interface, restoring behavior we had in .NET 8.

Microsoft Reviewers: Open in CodeFlow

@lonitra lonitra added 🚫 * NO-MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Jan 10, 2025
@lonitra lonitra requested a review from a team as a code owner January 10, 2025 00:41
@codecov
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 77.95276% with 28 lines in your changes missing coverage. Please review.

Project coverage is 74.83126%. Comparing base (aeb1e35) to head (f629c98).
Report is 3 commits behind head on release/9.0.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           release/9.0      #12745         +/-   ##
=====================================================
- Coverage     74.83635%   74.83126%   -0.00509%     
=====================================================
  Files             3022        3022                 
  Lines           630322      630413         +91     
  Branches         46798       46798                 
=====================================================
+ Hits            471710      471746         +36     
- Misses          155224      155277         +53     
- Partials          3388        3390          +2     
Flag Coverage Δ
Debug 74.83126% <77.95276%> (-0.00509%) ⬇️
integration 18.01343% <30.00000%> (-0.01513%) ⬇️
production 47.86030% <90.00000%> (-0.01155%) ⬇️
test 97.00090% <76.92308%> (-0.00804%) ⬇️
unit 44.87235% <90.00000%> (-0.03178%) ⬇️

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

@lonitra lonitra added servicing-approved .NET Shiproom approved the PR for merge and removed waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) servicing-consider .NET Shiproom label indicating a PR seeks to enter into a branch under Tell-Mode criteria labels Jan 10, 2025
@lonitra lonitra merged commit f96a54f into dotnet:release/9.0 Jan 14, 2025
@lonitra lonitra deleted the backport12738 branch January 14, 2025 17:06
@JeremyKuhne JeremyKuhne removed the 🚫 * NO-MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Jan 14, 2025
@Tanya-Solyanik Tanya-Solyanik added this to the 9.0.2 milestone Jan 15, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Feb 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

servicing-approved .NET Shiproom approved the PR for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants