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

Convert UIA Enum to Cswin32 #10081

Merged
merged 7 commits into from
Oct 12, 2023
Merged

Convert UIA Enum to Cswin32 #10081

merged 7 commits into from
Oct 12, 2023

Conversation

lonitra
Copy link
Member

@lonitra lonitra commented Oct 9, 2023

Removes UIA enum in preparation for more changes relating to converting COM interfaces on AccessibleObject.

Microsoft Reviewers: Open in CodeFlow

@ghost ghost assigned lonitra Oct 9, 2023
@ghost ghost added the draft draft PR label Oct 10, 2023

namespace Windows.Win32.UI.Accessibility;

internal enum UIA_CONTROLTYPE_ID : int
Copy link
Member Author

@lonitra lonitra Oct 11, 2023

Choose a reason for hiding this comment

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

Although Cswin32 has a defined UIA_CONTROLTYPE_ID, it is defined as uint, which causes Narrator to give incorrect announcements. This may be a consequence of having partially converted Cswin32 definitions. IRawElementProviderSimple is the primary interface that relies on the UIA enum and this hasn't been converted. For now, we will add this version of UIA_CONTROLTYPE_ID definition and revisit whether or not UIA_CONTROLTYPE_ID being defined as a uint enum in Cswin32 is a bug when we convert IRawElementProviderSimple to Cswin32, which is the next step.

Copy link
Member

Choose a reason for hiding this comment

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

All of these enums should be signed ints per docs and the idl file. This one is the easiest to test. We have to return tha documented type from GetPropertyValue methods becasue this is what the accessibility tools are expecting. We can't expect accessibility tools coded to be resilient and try to cast uint to int

Copy link
Member Author

Choose a reason for hiding this comment

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

Added UIA_PATTERN_ID and UIA_PROPERTY_ID and made them int enums. I will follow up filing an issue with metadata to make all the enums signed ints.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you! Once you have the issue, please reference it in this file.

Copy link
Member Author

Choose a reason for hiding this comment

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

@lonitra lonitra marked this pull request as ready for review October 11, 2023 20:09
@lonitra lonitra requested a review from a team as a code owner October 11, 2023 20:09
@lonitra lonitra removed the draft draft PR label Oct 11, 2023
@KlausLoeffelmann
Copy link
Member

Looks like a pretty mechanical change. Did you use find/replace or regex to swap the consts out?
I did spot checks, and everything looks plausible.

@lonitra
Copy link
Member Author

lonitra commented Oct 11, 2023

Looks like a pretty mechanical change. Did you use find/replace or regex to swap the consts out? I did spot checks, and everything looks plausible.

I used find/replace whenever there were multiple of the same UIA type (UIA_PROPERTY_TYPE, UIA_PATTERN_TYPE, etc.) that needed to be changed in a file, otherwise I manually converted it if there was only one UIA type to convert.

Copy link
Member

@Tanya-Solyanik Tanya-Solyanik 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, but some test data can be simplified a little by removing casts to an int. Look for (int)UIA_ pattern


namespace Windows.Win32.UI.Accessibility;

internal enum UIA_CONTROLTYPE_ID : int
Copy link
Member

Choose a reason for hiding this comment

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

Thank you! Once you have the issue, please reference it in this file.

[InlineData((int)UIA_PATTERN_ID.UIA_TogglePatternId)]
[InlineData((int)UIA_PATTERN_ID.UIA_InvokePatternId)]
[InlineData((int)UIA_PATTERN_ID.UIA_LegacyIAccessiblePatternId)]
[InlineData((int)UIA_PATTERN_ID.UIA_ValuePatternId)]
public void CheckedListBoxItemAccessibleObject_IsPatternSupported_ReturnsExpected(int patternId)
Copy link
Member

Choose a reason for hiding this comment

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

Now we can use the new enum type instead of the int in the parameter type, (UIA_PATTERN_ID patternId). This will let us get rid of the casts in the inline data.

This comment is applicable to multiple tests in this PR.

? (int)UiaCore.UIA.ComboBoxControlTypeId
: (int)UiaCore.UIA.DataItemControlTypeId;
? (int)UIA_CONTROLTYPE_ID.UIA_ComboBoxControlTypeId
: (int)UIA_CONTROLTYPE_ID.UIA_DataItemControlTypeId;

yield return new object[] { displayStyle, cellIsEdited, expectedControlType };
Copy link
Member

Choose a reason for hiding this comment

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

This data can return UIA_CONTROLTYPE_ID instead of an int too.

Copy link
Member

@Tanya-Solyanik Tanya-Solyanik left a comment

Choose a reason for hiding this comment

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

Feel free to merge as is and follow up on the comments in another PR

@lonitra
Copy link
Member Author

lonitra commented Oct 12, 2023

Will follow up on these comments in the next PR

@lonitra lonitra merged commit 7c5cde7 into dotnet:main Oct 12, 2023
9 checks passed
@lonitra lonitra deleted the uiaenum branch October 12, 2023 18:01
@ghost ghost locked as resolved and limited conversation to collaborators Nov 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants