Implement StandardAccessble RCW and AccessibleObject CCW #7145

kant2002 · 2022-05-06T06:07:31Z

I realize that EnumVariantWrapper RCW is essentially always implement 3 interfaces which is needed
IEnumVariant, IAccessible and IOleWindow
Some internal things of Win32 prevent me using just single call to CreateStdAccessibleObject
Also add CCW for AccessibleObject

With this change you can run WinForms application with disabled built-in COM except following controls:

RichTextBox
WebBrowser
AxHost

That's important step forward #5163

Microsoft Reviewers: Open in CodeFlow

RussKie · 2022-05-06T06:15:40Z

I realize that EnumVariantWrapper RCW is essentially always implement 3 interfaces which is needed
IEnumVariant, IAccessible and IOleWindow
Some internal things of Win32 prevent me using just single call to CreateStdAccessibleObject

Could you please elaborate on what led to the realisation, and what "internal things of Win32" scenarios led to this change? It would be great to capture all of these details, and may be add tests too.

kant2002 · 2022-05-06T06:28:58Z

"internal things of Win32" => ComObject which returned from https://docs.microsoft.com/en-us/windows/win32/api/oleacc/nf-oleacc-createstdaccessibleobject#parameters . here declared that parameters can be one of 4 interfaces. IID_IAccessible, IID_IDispatch, IID_IEnumVARIANT, or IID_IUnknown

Then when I probe for supported interfaces in WinFormsComWrappers.CreateObject all objects supports both IAccessible and IEnumVariant. I did try to simplify things, but conversion with ComObject does not work so I abandon that path. I would try to remove duplicate calls to CreateStdAccessibleObject and return back.

kant2002 · 2022-05-06T07:26:38Z

Okay. I remove duplicate calls to CreateStdAccessibleObject which just ask for different Com interfaces, so I really do not understand why there was two of them.

Only was to access internal IAccessible which I see is for Non-client area accessible objects. If somebody can tell me more about this scenario in accessibility, and how I can test more in that area?

Otherwise I think this is good.

src/System.Windows.Forms/src/System/Windows/Forms/ComboBox.ComboBoxChildNativeWindow.cs

src/System.Windows.Forms/tests/InteropTests/AccessibleObjectTests.cs

...System.Windows.Forms.Primitives/src/Interop/WinFormsComWrappers.StandardAccessibleWrapper.cs

kant2002 · 2022-05-10T14:13:09Z

@RussKie I think I address all questions. Am I?

kant2002 · 2022-05-16T03:18:46Z

@RussKie please take a look at this one too.

RussKie

👍 I'm getting better at this 😄
Is there some way we can verify we're not leaking objects?

@AaronRobinsonMSFT may I get you to review as well, when you have a chance?

...System.Windows.Forms.Primitives/src/Interop/WinFormsComWrappers.StandardAccessibleWrapper.cs

src/System.Windows.Forms/src/System/Windows/Forms/AccessibleObject.cs

This is successfully catches bug in the dotnet#7145 implementation

kant2002 · 2022-05-18T15:53:07Z

Please do not approve. I found test case #7191 which trigger bug with current implementation, and except much more code to make that happens. Overall I hate this AccessibleObject thing.

This is successfully catches bug in the dotnet#7145 implementation

kant2002 · 2022-05-18T19:01:25Z

On latest commit I fix issue and what's important this PR make WinForms finally working without built-in COM.
There some obvious limitations, like Clipboard which I do not finish, and RichTextBox for which I have code, but it's not published yet.

If this PR pass review, it would be time for celebration.

Tanya-Solyanik · 2022-05-18T07:56:57Z

src/System.Windows.Forms/src/System/Windows/Forms/AccessibleObject.cs

@@ -1749,31 +1749,31 @@ protected void UseStdAccessibleObjects(IntPtr handle)

        protected void UseStdAccessibleObjects(IntPtr handle, int objid)
        {
-            object? acc = null;
-            UnsafeNativeMethods.CreateStdAccessibleObject(
+            Guid IID_IAccessible = IID.IAccessible;


Name of this local does not follow conventions, You used iAccessibleId elsewhere. And the same applies to IEnumVariant

Tanya-Solyanik · 2022-05-18T07:59:39Z

src/System.Windows.Forms/tests/InteropTests/AccessibleObjectTests.cs

+                DropDownStyle = ComboBoxStyle.DropDown
+            };
+            control.CreateControl();
+            ComboBox.ComboBoxAccessibleObject accessibleObject = new ComboBox.ComboBoxAccessibleObject(control);


Consider new (control).

This is successfully catches bug in the #7145 implementation I need just call to `GetFocused`, but if some additional check can be made, I can add them.

kant2002 · 2022-05-25T02:13:59Z

@RussKie NativeAOT needs you :)

RussKie · 2022-05-25T03:45:35Z

Apologies, I've been slammed with other work

RussKie

👍

RussKie · 2022-05-25T03:50:06Z

src/System.Windows.Forms.Primitives/src/Interop/UiaCore/Interop.UiaDisconnectProvider.cs

+        private static extern HRESULT UiaDisconnectProvider(IntPtr provider);
+        public static HRESULT UiaDisconnectProvider(IRawElementProviderSimple provider)


[nit] Missing blank line

src/System.Windows.Forms.Primitives/src/Interop/UiaCore/Interop.UiaHostProviderFromHwnd.cs

...System.Windows.Forms.Primitives/src/Interop/UiaCore/Interop.UiaRaiseStructureChangedEvent.cs

src/System.Windows.Forms.Primitives/src/Interop/UiaCore/Interop.UiaReturnRawElementProvider.cs

...dows.Forms.Primitives/src/Interop/WinFormsComWrappers.IRawElementProviderFragmentRootVtbl.cs

RussKie · 2022-05-25T04:09:22Z

...System.Windows.Forms.Primitives/src/Interop/WinFormsComWrappers.StandardAccessibleWrapper.cs

+                ((delegate* unmanaged<IntPtr, Oleaut32.VARIANT, IntPtr *, HRESULT>)(*(*(void***)_accessibleInstance + 9)))
+                    (_accessibleInstance, childIdVar, &value).ThrowIfFailed();
+                return value == IntPtr.Zero ? null : Marshal.GetObjectForIUnknown(value);
+            }


Just to confirm I understand this correctly - when the return object will be GC'ed, value will get released. Is this correct?

Yes. From what I understand Marshal.GetObjectForIUnknown creates same RCW as built-in COM interop.

src/System.Windows.Forms.Primitives/src/Interop/WinFormsComWrappers.cs

src/System.Windows.Forms/tests/InteropTests/AccessibleObjectTests.cs

RussKie · 2022-05-25T04:21:22Z

@AaronRobinsonMSFT if you could spare few minutes for another pass, it'd be great. Mainly to see if we're leaking anything.

Because I provide AccessibleObject via WM_OBJECT I have to provide 3 interfaces IAccessible, IDispatch and IEnumVariant. But we rely on the IOleWindowVtbl Also I discover incorrect memory definition in ILegacyIAccessibleProvider

RussKie · 2022-07-18T08:40:52Z

I've just got back from being OOF, and looks like you've wrestled the tests over. I'm a bit swamped atm, but I'll try to get this reviewed by the EOW.

RussKie · 2022-07-18T08:42:48Z

@Olina-Zhang can you please schedule a test run for this change?

Olina-Zhang · 2022-07-18T10:34:03Z

@Olina-Zhang can you please schedule a test run for this change?

@kant2002 @RussKie I quickly tested the updated change, still have issue in here:

Our automation cases hit the issue about missing DoDragDrop test method, it seems your PR is not based on the latest main after the PR: Add support for drag images and drop descriptions #6576 merged
Picked up comboBox control to test in Accessibility tools: Accessibiltiy Insight, Inspect and Narrator, these tools cannot focus on or announce child elements automatically.

AccessibilityInsight result in this PR change:

Inspect result in this PR change: UIA tree cannot updated automatically, this effects Narrator announcement behavior.

kant2002 · 2022-07-18T10:50:55Z

Thanks! Will take a look and rebase on latest main

ghost · 2022-11-16T04:02:17Z

This submission has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 14 days.

It will be closed if no further activity occurs within 7 days of this comment.

dreddy-work · 2022-11-23T00:25:38Z

prevent bot from closing it.

ghost · 2022-12-07T01:02:32Z

This submission has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 14 days.

It will be closed if no further activity occurs within 7 days of this comment.

dreddy-work · 2023-06-05T17:19:43Z

ping.

ghost · 2023-06-19T19:02:08Z

This submission has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 14 days.

It will be closed if no further activity occurs within 7 days of this comment.

JeremyKuhne · 2023-10-04T18:23:24Z

Thanks for the effort here @kant2002- it helped get us in gear to make the COM changes we already made for .NET 8. @lonitra is handling AccessibleObject with CsWin32 and ComWrappers in .NET 9.

kant2002 · 2023-10-04T18:28:14Z

I love this news :)

kant2002 requested a review from a team as a code owner May 6, 2022 06:07

ghost assigned kant2002 May 6, 2022

RussKie reviewed May 9, 2022

View reviewed changes

RussKie added the waiting-author-feedback The team requires more information from the author label May 9, 2022

ghost removed the waiting-author-feedback The team requires more information from the author label May 9, 2022

RussKie reviewed May 16, 2022

View reviewed changes

...System.Windows.Forms.Primitives/src/Interop/WinFormsComWrappers.StandardAccessibleWrapper.cs Outdated Show resolved Hide resolved

...System.Windows.Forms.Primitives/src/Interop/WinFormsComWrappers.StandardAccessibleWrapper.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT reviewed May 16, 2022

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/AccessibleObject.cs Outdated Show resolved Hide resolved

AaronRobinsonMSFT reviewed May 16, 2022

View reviewed changes

src/System.Windows.Forms/src/System/Windows/Forms/AccessibleObject.cs Outdated Show resolved Hide resolved

RussKie previously approved these changes May 16, 2022

View reviewed changes

kant2002 dismissed RussKie’s stale review via 2d9b5b7 May 18, 2022 13:19

kant2002 added a commit to kant2002/winforms that referenced this pull request May 18, 2022

Test that numeric up-down successfully focused

c3c1d26

This is successfully catches bug in the dotnet#7145 implementation

kant2002 mentioned this pull request May 18, 2022

Test that numeric up-down successfully focused #7191

Merged

kant2002 added a commit to kant2002/winforms that referenced this pull request May 18, 2022

Test that numeric up-down successfully focused

10409eb

This is successfully catches bug in the dotnet#7145 implementation

Tanya-Solyanik reviewed May 18, 2022

View reviewed changes

kant2002 changed the title ~~Implement StandardAccessble object~~ Implement StandardAccessble RCW and AccessibleObject CCW May 21, 2022

ghost pushed a commit that referenced this pull request May 24, 2022

Test that numeric up-down successfully focused (#7191)

d591fb4

This is successfully catches bug in the #7145 implementation I need just call to `GetFocused`, but if some additional check can be made, I can add them.

kant2002 force-pushed the kant/standard-accessible-obect branch from f4e1713 to bfc9519 Compare May 24, 2022 20:16

RussKie reviewed May 25, 2022

View reviewed changes

RussKie added the waiting-author-feedback The team requires more information from the author label May 25, 2022

ghost removed the waiting-author-feedback The team requires more information from the author label May 25, 2022

kant2002 added 3 commits July 5, 2022 12:48

Implement StdAccessibleObject

58a621d

Because I provide AccessibleObject via WM_OBJECT I have to provide 3 interfaces IAccessible, IDispatch and IEnumVariant. But we rely on the IOleWindowVtbl Also I discover incorrect memory definition in ILegacyIAccessibleProvider

formatting

18d45b6

onenmore try

7d42479

kant2002 force-pushed the kant/standard-accessible-obect branch from 32ad267 to 7d42479 Compare July 5, 2022 06:49

runfoapp bot mentioned this pull request Jul 5, 2022

Flaky System.Windows.Forms.Tests.ToolStripTests.ToolStrip_WndProc_InvokeMouseActivate_Success #7333

Closed

RussKie added waiting-for-testing The PR is awaiting manual testing by the primary team; no action is yet required from the author(s) waiting-review This item is waiting on review by one or more members of team labels Jul 18, 2022

ghost added the no-recent-activity label Nov 16, 2022

ghost removed the no-recent-activity label Nov 23, 2022

ghost added the no-recent-activity label Dec 7, 2022

ghost removed the no-recent-activity label Jun 5, 2023

ghost added the no-recent-activity label Jun 19, 2023

JeremyKuhne closed this Oct 4, 2023

ghost removed no-recent-activity waiting-author-feedback The team requires more information from the author labels Oct 4, 2023

ghost locked as resolved and limited conversation to collaborators Nov 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement StandardAccessble RCW and AccessibleObject CCW #7145

Implement StandardAccessble RCW and AccessibleObject CCW #7145

kant2002 commented May 6, 2022 •

edited

Loading

RussKie commented May 6, 2022

kant2002 commented May 6, 2022

kant2002 commented May 6, 2022

kant2002 commented May 10, 2022

kant2002 commented May 16, 2022

RussKie left a comment

kant2002 commented May 18, 2022

kant2002 commented May 18, 2022

Tanya-Solyanik May 18, 2022

Tanya-Solyanik May 18, 2022

kant2002 commented May 25, 2022

RussKie commented May 25, 2022

RussKie left a comment

RussKie May 25, 2022

RussKie May 25, 2022

kant2002 May 25, 2022

RussKie commented May 25, 2022

RussKie commented Jul 18, 2022

RussKie commented Jul 18, 2022

Olina-Zhang commented Jul 18, 2022

kant2002 commented Jul 18, 2022

ghost commented Nov 16, 2022

dreddy-work commented Nov 23, 2022

ghost commented Dec 7, 2022

dreddy-work commented Jun 5, 2023

ghost commented Jun 19, 2023

JeremyKuhne commented Oct 4, 2023

kant2002 commented Oct 4, 2023

		private static extern HRESULT UiaDisconnectProvider(IntPtr provider);
		public static HRESULT UiaDisconnectProvider(IRawElementProviderSimple provider)

Implement StandardAccessble RCW and AccessibleObject CCW #7145

Implement StandardAccessble RCW and AccessibleObject CCW #7145

Conversation

kant2002 commented May 6, 2022 • edited Loading

Microsoft Reviewers: Open in CodeFlow

RussKie commented May 6, 2022

kant2002 commented May 6, 2022

kant2002 commented May 6, 2022

kant2002 commented May 10, 2022

kant2002 commented May 16, 2022

RussKie left a comment

Choose a reason for hiding this comment

kant2002 commented May 18, 2022

kant2002 commented May 18, 2022

Tanya-Solyanik May 18, 2022

Choose a reason for hiding this comment

Tanya-Solyanik May 18, 2022

Choose a reason for hiding this comment

kant2002 commented May 25, 2022

RussKie commented May 25, 2022

RussKie left a comment

Choose a reason for hiding this comment

RussKie May 25, 2022

Choose a reason for hiding this comment

RussKie May 25, 2022

Choose a reason for hiding this comment

kant2002 May 25, 2022

Choose a reason for hiding this comment

RussKie commented May 25, 2022

RussKie commented Jul 18, 2022

RussKie commented Jul 18, 2022

Olina-Zhang commented Jul 18, 2022

kant2002 commented Jul 18, 2022

ghost commented Nov 16, 2022

dreddy-work commented Nov 23, 2022

ghost commented Dec 7, 2022

dreddy-work commented Jun 5, 2023

ghost commented Jun 19, 2023

JeremyKuhne commented Oct 4, 2023

kant2002 commented Oct 4, 2023

kant2002 commented May 6, 2022 •

edited

Loading