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

Changed type of the handle parameter from IntPtr to HWND in the Control.ReleaseUiaProvider method #8351

Conversation

NikitaSemenovAkvelon
Copy link
Contributor

@NikitaSemenovAkvelon NikitaSemenovAkvelon commented Dec 8, 2022

Fixes #8217

Proposed changes

  • Changed type to HWND.
  • Refactored Control.HWNDInternal property to remove extra convert.
  • Used HWND value instead of Handle.
  • Used HWND.Null value instead of IntPtr.Zero.

Customer Impact

  • Method becomes more clear and an argument type is unified.

Regression?

  • No

Risk

  • Minimal

Test methodology

  • Manual
  • Unit tests

Accessibility testing

  • Inspect

Test environment(s)

.NET SDK:
Version: 8.0.100-alpha.1.22512.5
Commit: 1b80461e45

Runtime Environment:
OS Name: Windows
OS Version: 10.0.22621

Microsoft Reviewers: Open in CodeFlow

@NikitaSemenovAkvelon NikitaSemenovAkvelon requested a review from a team as a code owner December 8, 2022 07:06
@NikitaSemenovAkvelon NikitaSemenovAkvelon added the waiting-review This item is waiting on review by one or more members of team label Dec 8, 2022
@elachlan
Copy link
Contributor

elachlan commented Dec 8, 2022

LGTM 🚀

My only nit would be to have two methods, one for the HWNDInternal release and the other for the HWND.Null release. That way there is no way to call a release with anything else.

@NikitaSemenovAkvelon
Copy link
Contributor Author

My only nit would be to have two methods, one for the HWNDInternal release and the other for the HWND.Null release. That way there is no way to call a release with anything else.

I'm not sure if it is a good idea because we have overriding of this method in many classes.
If we split it into two parameterless methods we will have to have a main with the parameter for overriding.

Copy link
Member

@lonitra lonitra left a comment

Choose a reason for hiding this comment

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

LGTM!

@lonitra lonitra added the ready-to-merge PRs that are ready to merge but worth notifying the internal team. label Dec 20, 2022
@Tanya-Solyanik Tanya-Solyanik removed the waiting-review This item is waiting on review by one or more members of team label Dec 22, 2022
Copy link
Contributor

@dkazennov dkazennov left a comment

Choose a reason for hiding this comment

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

Looks good!

@dreddy-work dreddy-work merged commit 7fd92af into dotnet:main Jan 4, 2023
@ghost ghost added this to the 8.0 Preview1 milestone Jan 4, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Feb 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready-to-merge PRs that are ready to merge but worth notifying the internal team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unifying an argument in the ReleaseUiaProvider method to HWND.
7 participants