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

The application crashes when trying to get data about the DropDownList in the PropertyGrid through the Inspect #6165

Closed
SergeySmirnov-Akvelon opened this issue Nov 11, 2021 · 3 comments · Fixed by #6192
Assignees

Comments

@SergeySmirnov-Akvelon
Copy link
Contributor

SergeySmirnov-Akvelon commented Nov 11, 2021

.NET Core Version:

  • 7.0.0-alpha.1.21558.10

Have you experienced this same bug with .NET Framework?:

Repro steps:

  1. Create an app with a PropertyGrid and button.
  2. Set Button as SelectedObject property of PropertyGrid.
  3. Run the app.
  4. Run the Inspect.
  5. Set focus to Accessible Role dropdown list.

Actual behavior:
The Application crashes:
Issue-6165

Expected behavior:
The application should not crash.

SergeySmirnov-Akvelon added a commit to SergeySmirnov-Akvelon/winforms that referenced this issue Nov 11, 2021
…DownList in the PropertyGrid through the Inspect dotnet#6165

The issue is reproduced because we throw an NotSupportedException when user tries to get the "RuntimeId" properties for the base AccessibleObject.

Added return of Array.Empty for base class
@ghost ghost added the 🚧 work in progress Work that is current in progress label Nov 11, 2021
SergeySmirnov-Akvelon added a commit to SergeySmirnov-Akvelon/winforms that referenced this issue Nov 11, 2021
…DownList in the PropertyGrid through the Inspect dotnet#6165

The issue is reproduced because we throw an NotSupportedException when user tries to get the "RuntimeId" properties for the base AccessibleObject.

Added return of Array.Empty for base class

Remove "AccessibleObjectRuntimeIdNotSupported" from string resources
@WPMGPRoSToTeMa WPMGPRoSToTeMa self-assigned this Nov 16, 2021
WPMGPRoSToTeMa added a commit to WPMGPRoSToTeMa/winforms that referenced this issue Nov 16, 2021
WPMGPRoSToTeMa added a commit to WPMGPRoSToTeMa/winforms that referenced this issue Dec 2, 2021
WPMGPRoSToTeMa added a commit to WPMGPRoSToTeMa/winforms that referenced this issue Dec 2, 2021
Fixes dotnet#6165. `AccessibleObject.RuntimeId` was previously changed to assert and throw an excepton to avoid subclasses with non-overriden `RuntimeId` property (see dotnet#5638 for details). But it was overlooked that `AccessibleObject` is also used to wrap system accessible objects. Because of that you may get a crash or hit an assertion (with debugger attached) when running a debug version of the code. This doesn't affect users of WinForms since the SDK is compiled in release mode.

To fix the issue a separate subclass of `AccessibleObject` may be created. But this will probably be a big and complex change. Instead of that we just can add a branch for system wrapper to `RuntimeId` property.

An empty array can be used as a `RuntimeId` value for system wrappers. This will cause the UIAutomationCore to perform field-by-field comparison instead of `RuntimeId` comparison. It doesn't look as a good option since field-by-field comparison may be bad for performance and also it will be better to not rely on the UIAutomationCore internal behavior.

Hash code value was applied as a valid `RuntimeId` for system wrappers. It is better than an empty array since it doesn't cause any performance penalties and is explicit so we don't rely on UIAutomationCore behavior.
@WPMGPRoSToTeMa
Copy link
Contributor

It is crashing because you're running application without debugger attached. Otherwise it will pause at Debug.Fail just like hitting a regular breakpoint. Also there will be no issue if you will run it compiled in release mode no matter with or without debugger attached since assertions aren't compiled in release mode.

Tanya-Solyanik pushed a commit that referenced this issue Dec 3, 2021
…6192)

Fixes #6165. `AccessibleObject.RuntimeId` was previously changed to assert and throw an excepton to avoid subclasses with non-overriden `RuntimeId` property (see #5638 for details). But it was overlooked that `AccessibleObject` is also used to wrap system accessible objects. Because of that you may get a crash or hit an assertion (with debugger attached) when running a debug version of the code. This doesn't affect users of WinForms since the SDK is compiled in release mode.

To fix the issue a separate subclass of `AccessibleObject` may be created. But this will probably be a big and complex change. Instead of that we just can add a branch for system wrapper to `RuntimeId` property.

An empty array can be used as a `RuntimeId` value for system wrappers. This will cause the UIAutomationCore to perform field-by-field comparison instead of `RuntimeId` comparison. It doesn't look as a good option since field-by-field comparison may be bad for performance and also it will be better to not rely on the UIAutomationCore internal behavior.

Hash code value was applied as a valid `RuntimeId` for system wrappers. It is better than an empty array since it doesn't cause any performance penalties and is explicit so we don't rely on UIAutomationCore behavior.
@ghost ghost removed the 🚧 work in progress Work that is current in progress label Dec 3, 2021
@RussKie
Copy link
Member

RussKie commented Dec 6, 2021

It is crashing because you're running application without debugger attached. Otherwise it will pause at Debug.Fail just like hitting a regular breakpoint. Also there will be no issue if you will run it compiled in release mode no matter with or without debugger attached since assertions aren't compiled in release mode.

I don't agree with this view angle - an app must never crash in Debug or Release mode unless it is a developer-originated error that the developer can fix. And it is never ok to accept crashes in Debug because we don't in Release - by doing this we are ignoring the root cause.

@Ashley-Li
Copy link

Verified this issue with .NET 7.0.100-alpha.1.21568.2, it is fixed. The application does not crash when setting focus to Accessible Role dropdown list.
6165-verify

@Lydia-Shi Lydia-Shi added this to the 7.0 Preview1 milestone Jan 12, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 11, 2022
@RussKie RussKie removed this from the 7.0 Preview1 milestone May 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.