search

CommunityToolkit / WindowsCommunityToolkit

The Windows Community Toolkit is a collection of helpers, extensions, and custom controls. It simplifies and demonstrates common developer tasks building .NET apps with UWP and the Windows App SDK / WinUI 3 for Windows 10 and Windows 11. The toolkit is part of the .NET Foundation.
https://docs.microsoft.com/windows/communitytoolkit/
Other
5.89k stars 1.37k forks source link

DataGridRows are not where they should be in release mode #3463

Open DanielGlick opened 4 years ago

DanielGlick commented 4 years ago

I had opened a previous issue about DataGridRows not being accessible in release mode (https://github.com/windows-toolkit/WindowsCommunityToolkit/issues/3079). Sadly, I was not active in the issue and it got closed without being fixed. I did a little more digging and came up with good steps to reproduce the issue.

Describe the bug

Steps to Reproduce

ReleaseDataGrid.zip

  1. Launch attached project in release mode
  2. Open Accessibility Insights for Windows
  3. Hover over the first row
  4. In Accessibility insights, the tree looks like the first screenshot

Expected behavior

Screenshots

This first image is the tree of an app in release mode. You can see that the parent of data item (The row) is the window. This is in release mode image

The second image here is the tree of the same app in debug mode. In debug mode the parent of data item is the data grid (this is what I would expect) image

Environment

NuGet Package(s) and Versions: Microsoft.NETCore.UniversalWindowsPlatform, version 6.2.10 Microsoft.Toolkit.Uwp.UI, version 6.1.1 Microsoft.Toolkit.Uwp.UI.Controls.DataGrid, version 6.1.1

Windows 10 Build Number:

App min and target version:

Device form factor:

Visual Studio

ghost commented 4 years ago

Hello DanielGlick45, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

Kyaa-dost commented 4 years ago

Thanks @DanielGlick45! Looping in the team @anawishnoff and @RBrid ⬆️

anawishnoff commented 4 years ago

Hmm, this is an interesting issue, thanks for reporting it. I'm not sure why the tree would change in debug vs release mode. @RBrid calling upon your wealth of knowledge once again...

DanielGlick commented 4 years ago

@anawishnoff is there any update here? It would be really nice to have this resolved. This is holding us up from writing UITests that use the DataGrid

RBrid commented 4 years ago

I am seeing the same issue using the Windows Community Toolkit's built-in SampleApp (its DataGrid control test page).

Debug screenshot: DataGridAccessibilityDebug1

Release screenshot: DataGridAccessibilityRetail1

The 'dataGrid' appears only in the Debug build. I could not figure out why though.

If someone wants to help, there is a DEBUG_AUTOMATION flag (and DEBUG) that can be turned on to output debugging information while the SampleApp and 'Accessibility Insights for Windows' are running:

DataGridAccessibilityOutput

Debugging outputs of this nature probably need to be added to gather more information:

#if DEBUG_AUTOMATION
            Debug.WriteLine("DataGridAutomationPeer.GetClassNameCore returns " + classNameCore);
#endif

I probably won't be able to look into this further anytime soon.

jamers99 commented 4 years ago

Our company has a very high interest in making this work in Release. @DanielGlick45 and our team will be looking into fixing this. Any help would be greatly appreciated! Thanks for the tips @RBrid.

RBrid commented 4 years ago

One of our automation gurus provided this info that could be helpful: "I do not see that exact result, but I would check DataGridRowsPresenter and its functionality to return children.

From a very quick look, it looks like GetChildren of the row presenter (DataGridRowsPresenter) does not return any children? Quite likely the bug is due to that, the data items inside the grid fall back to the window as their parent (just like you would see in XAML, basically, if you hid controls)."

I would use his comments if I had time to look into this further.

DanielGlick commented 4 years ago

@RBrid. A few things I have found...

  1. I am not seeing a method GetChildren on DataGridRowsPresenter.
  2. I noticed is that in Accessibility Insights if I listen to events and click on a row I see the DataGrid in the list of events getting focus
  3. In accessibility insights, if I find the row (or data item as it shows in Accessibility Insights), the SelectionItemPatter.SelectionContainer is the data grid. Similarly if I select the control inside the data item I see GridItemPattern.Parent is the data grid. Im thinking what @RBrid said above could be correct, but I am not sure how to verify that or debug it. Any thoughts/ideas?
RBrid commented 4 years ago

@DanielGlick45, sorry for the confusion, he was talking about automation peer children, as in DataGridRowsPresenterAutomationPeer's returned children. Is DataGridRowsPresenterAutomationPeer.GetChildrenCore() returning children representing the rows?

DanielGlick commented 4 years ago

@RBrid, do you have any tips for debugging this in release mode? I am finding that it is very difficult to get consistent debugging in release. Debug is fine, but in release I set breakpoints and it doesnt always hit them, sometimes the breakpoints are disabled with a warning like "No symbols have been loaded for this document", just different things like that.

Anyway. GetChildrenCore on DataGridRowsPresenter returns 117 rows in the sample app. If I select the datagrid from AccessibilityInsights I can see that the GridPattern.RowCount is also 117, so I think everything is good there.

Any other ideas on what could be causing the bug?

jamers99 commented 4 years ago

@DanielGlick45 @alisaesh one thing to check would be if it works in debug with .NET Native enabled for the projects. That would tell you if the problem is in the DataGrid code, or if the issue is with .NET Native compilation.

alisaesh commented 4 years ago

A few observations from debugging: I seems as though it doesn't find the row at all in release unless you click into it. I tried to click into a row in a UI test in release and the test can't find the row. It would appear that you need to be able to hover over the row and find it to be able to use it in UI tests. As @DanielGlick45 stated, DataGridRowsPresentor.GetChildrenCore() as far as I can tell is returning the correct value. @RBrid Any ideas on what else I could debug here to find the issue?

When I hover over a datagrid row in release, this is all I am seeing: image

When I hover over a datagrid row in debug, I see the expected behavior: image

alisaesh commented 4 years ago

@jamers99 Enabling .NET Native in debug does reproduce the issue.

jamers99 commented 4 years ago

@RBrid see @alisaesh's comment. Looks like this issue is entirely in the .NET Native Compiler. Who should we reach out to? How do we reach out? If there are logs or dumps needed, we should be able to help out, but for now, our team is not working on this anymore.

tommcdon commented 4 years ago

Hi! To help us investigate the .NET native issue, please send us an ILC repro to dotnetnative@microsoft.com. This should give us what is needed to investigate next steps.

jamers99 commented 4 years ago

@alisaesh is this something you could do for @tommcdon?

alisaesh commented 4 years ago

Done.

jamers99 commented 4 years ago

@tommcdon I was debugging this and found that there is an exception being thrown in our app when I hover over a row.

System.Runtime.InteropServices.MissingInteropDataException
  HResult=0x80131500
  Message=System.Collections.Generic.IList`1[Windows.UI.Xaml.Automation.Peers.AutomationPeer] is missing interop type marshalling data. To enable interop type marshalling data, add a MarshalObject directive to the application rd.xml file. For more information, please visit http://go.microsoft.com/fwlink/?LinkID=393965
  Source=<Cannot evaluate the exception source>
  StackTrace:
<Cannot evaluate the exception stack trace>

image

I tried adding this to our application's Default.rd.xml file, but the same issue occurred <Type Name="System.Collections.Generic.IList{Windows.UI.Xaml.Automation.Peers.AutomationPeer}" Dynamic="Required All" />

image

Any thoughts? Am I doing something wrong in the .rd.xml file? Should this line be added to the Toolkit DataGrid project?

tommcdon commented 4 years ago

@jamers99 It looks like AutomationPeer is an object, and and so adding the MarshalObject="Required All" attribute should hopefully resolve the issue. Please take a look at the https://docs.microsoft.com/en-us/dotnet/framework/net-native/runtime-directives-rd-xml-configuration-file-reference for specifics on policy settings.

jamers99 commented 4 years ago

@jamers99 It looks like AutomationPeer is an object, and and so adding the MarshalObject="Required All" attribute should hopefully resolve the issue. Please take a look at the https://docs.microsoft.com/en-us/dotnet/framework/net-native/runtime-directives-rd-xml-configuration-file-reference for specifics on policy settings.

Thanks for the info! A few follow up questions...

  1. Should I be doing it for Type Name "Windows.UI.Xaml.Automation.Peers.AutomationPeer" or does it need to include the IList generic?
  2. Which project should specify this? Documentation is not clear on where these should be defined.
jamers99 commented 4 years ago

@tommcdon I used https://dotnet.github.io/native/troubleshooter/type.html to help me generate what I needed. Is this a supported tool? It seems it didn't generate the proper attributes.

image

tommcdon commented 4 years ago

@jamers99

  1. Should I be doing it for Type Name "Windows.UI.Xaml.Automation.Peers.AutomationPeer" or does it need to include the IList generic?

Great point! I should have looked at the exception more closely. You are correct that the IList is what the exception the specific error. If not too much trouble, I would try adding both to the rd.xml, and if it solves the problem try removing the AutomationPeer-specfic item.

  1. Which project should specify this? Documentation is not clear on where these should be defined.

Another great question. .NET native compiles the application and its references together into one binary, and so it will use the rd.xml in the UWP app csproj. You should be able to verify it is using it by enable diagnostic logging under Tools->Options->Projects and solutions->Build and Run and look for ilc.exe invocation parameters (usually imported via a response file).

I used https://dotnet.github.io/native/troubleshooter/type.html to help me generate what I needed. Is this a supported tool? It seems it didn't generate the proper attributes.

Yes this tool is supported. Great feedback that we need to add MissingInteropDataException support to it.

jamers99 commented 4 years ago

@tommcdon @RBrid so from the sound of it, we can resolve this issue in our app (I'm testing it out now), but is there a way that the toolkit library can fix it for everyone in any project?

This...

it will use the rd.xml in the UWP app csproj.

...makes it sound like it needs to be done per application?

jamers99 commented 4 years ago

By the way adding...

    <Type Name="System.Collections.Generic.IList{Windows.UI.Xaml.Automation.Peers.AutomationPeer}" MarshalObject="Required All" Dynamic="Required All" />

...fixed the problem for me! I now see the DataGrid as it is in Debug in the Accessibility Insights! Thanks so much for your help! I would like to see this fixed at the library level though, @RBrid do you think we would be able to do that?

ghost commented 3 years ago

Thanks DanielGlick45 for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

ghost commented 3 years ago

Thanks DanielGlick45 for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

ghost commented 3 years ago

Thanks DanielGlick45 for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

ghost commented 3 years ago

Thanks DanielGlick45 for patiently waiting while the team is still investigating the issue.

@anawishnoff @RBrid can we please have an update on this issue? Thanks

DanielGlick commented 2 years ago

@RBrid @anawishnoff

Is there any update here? I was going through my open issues and noticed this is still here. I would at least like to know if you are planning to fix this. If not I am in favor of closing it since on our end we have a suitable workaround.