WindowsCommunityToolkit icon indicating copy to clipboard operation
WindowsCommunityToolkit copied to clipboard

DataGridRows are not where they should be in release mode

Open DanielGlick opened this issue 3 years ago • 29 comments

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

  • DataGridRows show up on the tree differently in release mode than debug mode

  • [x] Is this bug a regression in the toolkit? If so, what toolkit version did you last see it work: I am fairly certain that this used to work, I dont know when it last did though

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

  • The row should show correctly in the tree, like in the second screenshot

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:

  • [ ] Fall Creators Update (16299)
  • [ ] April 2018 Update (17134)
  • [ ] October 2018 Update (17763)
  • [x] May 2019 Update (18362)
  • [ ] May 2020 Update (19041)
  • [ ] Insider Build (build number: )

App min and target version:

  • [ ] Fall Creators Update (16299)
  • [ ] April 2018 Update (17134)
  • [ ] October 2018 Update (17763)
  • [x] May 2019 Update (18362)
  • [x] May 2020 Update (19041)
  • [ ] Insider Build (xxxxx)

Device form factor:

  • [x] Desktop
  • [ ] Xbox
  • [ ] Surface Hub
  • [ ] IoT

Visual Studio

  • [ ] 2017 (version: )
  • [x] 2019 (version: 16.6.5)
  • [ ] 2019 Preview (version: )

DanielGlick avatar Sep 03 '20 02:09 DanielGlick

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 🙌

ghost avatar Sep 03 '20 02:09 ghost

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

Kyaa-dost avatar Sep 03 '20 16:09 Kyaa-dost

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...

anawishnoff avatar Sep 03 '20 20:09 anawishnoff

@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

DanielGlick avatar Sep 16 '20 18:09 DanielGlick

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.

RBrid avatar Sep 17 '20 01:09 RBrid

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.

jamers99 avatar Oct 06 '20 17:10 jamers99

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.

RBrid avatar Oct 06 '20 18:10 RBrid

@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?

DanielGlick avatar Oct 07 '20 18:10 DanielGlick

@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?

RBrid avatar Oct 08 '20 21:10 RBrid

@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?

DanielGlick avatar Oct 09 '20 00:10 DanielGlick

@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.

jamers99 avatar Oct 15 '20 15:10 jamers99

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 avatar Oct 15 '20 17:10 alisaesh

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

alisaesh avatar Oct 15 '20 18:10 alisaesh

@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.

jamers99 avatar Oct 19 '20 19:10 jamers99

Hi! To help us investigate the .NET native issue, please send us an ILC repro to [email protected]. This should give us what is needed to investigate next steps.

tommcdon avatar Oct 19 '20 22:10 tommcdon

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

jamers99 avatar Oct 21 '20 01:10 jamers99

Done.

alisaesh avatar Nov 06 '20 20:11 alisaesh

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

  • In Release
  • Attached the debugger
  • All CLR Exceptions enabled in "Exception Settings"
  • (I did not try this in the sample toolkit app)
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?

jamers99 avatar Nov 14 '20 00:11 jamers99

@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.

tommcdon avatar Nov 17 '20 21:11 tommcdon

@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 avatar Nov 17 '20 21:11 jamers99

@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

jamers99 avatar Nov 17 '20 21:11 jamers99

@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<AutomationPeer> 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.

tommcdon avatar Nov 17 '20 22:11 tommcdon

@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 avatar Nov 17 '20 22:11 jamers99

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?

jamers99 avatar Nov 17 '20 22:11 jamers99

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 avatar Apr 08 '21 18:04 ghost

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 avatar May 09 '21 00:05 ghost

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 avatar Jun 08 '21 06:06 ghost

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 avatar Jul 08 '21 06:07 ghost

@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.

DanielGlick avatar Jul 25 '22 19:07 DanielGlick