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

[CRASH] Android app crash at runtime when debugging #231

Open
carldebilly opened this issue May 6, 2022 · 7 comments
Open

[CRASH] Android app crash at runtime when debugging #231

carldebilly opened this issue May 6, 2022 · 7 comments
Assignees

Comments

@carldebilly
Copy link
Member

carldebilly commented May 6, 2022

Crash

I'm getting this error on Android:

System.InvalidOperationException: 'The Dependency Property [UriSource] is owned by [Microsoft.UI.Xaml.Controls.BitmapIcon] and cannot be used on [Microsoft.UI.Xaml.Controls.PathIcon]'

Steps

  1. Launch the app on Android with the debugger
  2. Login
  3. Select any task list
  4. Observe the crash with the provided error message.

Other details

The application won't crash if you're not debugging the application.

@carldebilly carldebilly changed the title [CRASH] Android app crash at runtime [CRASH] Android app crash at runtime when debugging May 6, 2022
@agneszitte agneszitte self-assigned this May 9, 2022
@agneszitte
Copy link
Contributor

agneszitte commented May 10, 2022

@carldebilly on my side when testing with an android emulator I have no crashes but I can't navigate to a task list. I'm stuck on the NavigationView. (cc @nickrandolph)
Let me know if it's the same for you.

@nickrandolph
Copy link
Contributor

@dr1rrb there's a NullReferenceException thrown on Android when navigating to the TaskListPage. It seems to be this line that's causing the issue (I commented out a lot of the xaml and when I enabled this line it started to throw the exception again).

<utu:AutoLayout Background="{Binding Entity.Value, Converter={StaticResource TaskListSurfaceBrushConverter}}">

Would you be able to take a look and see what's happening when Entity.Value is being called - the converter isn't being called, so I'm assuming it's an issue with calling Entity.Value

@dr1rrb
Copy link
Member

dr1rrb commented May 11, 2022

@agneszitte-nventive the navigation fails as some brushes are missing on Android. In the Page.Resources of the the TaskListPage, if you replace the brushes by the xaml below, you will be able to navigate.

		<SolidColorBrush Color="DeepPink" x:Key="DefaultTaskListSurfaceBrush" />
		<SolidColorBrush Color="DeepSkyBlue" x:Key="DefaultTaskListOnSurfaceBrush" />
		<SolidColorBrush Color="Chartreuse" x:Key="ImportantTaskListSurfaceBrush" />
		<SolidColorBrush Color="Orange" x:Key="ImportantTaskListOnSurfaceBrush" />

This is the issue your are observing @nickrandolph : As the converter TaskListSurfaceBrushConverter is resolved lazily and tries to resolve those resources, the error occurs only when we do have a Entity.Value.

But indeed when we pass those issues we do have crash reported by Carl.

Edit: @agneszitte-nventive @carldebilly Not sure who is responsible to import the resources nor if they should be automatically imported by the figma code gen ... I won't look at them, I'm investigating the cause of the crash.

@agneszitte
Copy link
Contributor

@agneszitte-nventive the navigation fails as some brushes are missing on Android. In the Page.Resources of the the TaskListPage, if you replace the brushes by the xaml below, you will be able to navigate.

		<SolidColorBrush Color="DeepPink" x:Key="DefaultTaskListSurfaceBrush" />
		<SolidColorBrush Color="DeepSkyBlue" x:Key="DefaultTaskListOnSurfaceBrush" />
		<SolidColorBrush Color="Chartreuse" x:Key="ImportantTaskListSurfaceBrush" />
		<SolidColorBrush Color="Orange" x:Key="ImportantTaskListOnSurfaceBrush" />

This is the issue your are observing @nickrandolph : As the converter TaskListSurfaceBrushConverter is resolved lazily and tries to resolve those resources, the error occurs only when we do have a Entity.Value.

But indeed when we pass those issues we do have crash reported by Carl.

Edit: @agneszitte-nventive @carldebilly Not sure who is responsible to import the resources nor if they should be automatically imported by the figma code gen ... I won't look at them, I'm investigating the cause of the crash.

Thanks a lot for investigating @dr1rrb !!
I will check for the brushes / imported resources

@agneszitte agneszitte assigned dr1rrb and unassigned dr1rrb May 11, 2022
@dr1rrb
Copy link
Member

dr1rrb commented May 11, 2022

About the crash itself, I noticed that the issue is due to the NavigationBar here :

<utu:NavigationBar Content="{Binding Entity.DisplayName}"

Even if I comment the PathIcon I get another exception (which also crash the app):

System.InvalidOperationException: 'The Dependency Property [Color] is owned by [Microsoft.UI.Xaml.Media.SolidColorBrush] and cannot be used on [Uno.Toolkit.UI.NavigationBar]'

Note: Even if I comment the material styles I still get the exception

I was not able to find where we wrongly set those properties in the app nor in Uno.Themes. My main suspect is https://github.com/unoplatform/uno/blob/7b4671c663cd71cac5f20565a8b48cf9f7c81606/src/Uno.UI/Controls/CommandBar/AppBarButtonRenderer.Android.cs#L55.

But anyway as this exception is raise only in debug (https://github.com/unoplatform/uno/blob/7b4671c663cd71cac5f20565a8b48cf9f7c81606/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs#L642) and the effect is probably just a minor perf hit, I don't see that as a urgent issue anymore and I will transfer the issue in the uno repo.

GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/AppBarButtonRenderer.Android.cs at 7b4671c663cd71cac5f20565a8b48cf9f7c81606 · uno...
GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/DependencyObjectStore.cs at 7b4671c663cd71cac5f20565a8b48cf9f7c81606 · unoplatfor...

@agneszitte
Copy link
Contributor

agneszitte commented May 13, 2022

About the crash itself, I noticed that the issue is due to the NavigationBar here :

<utu:NavigationBar Content="{Binding Entity.DisplayName}"

Even if I comment the PathIcon I get another exception (which also crash the app):

System.InvalidOperationException: 'The Dependency Property [Color] is owned by [Microsoft.UI.Xaml.Media.SolidColorBrush] and cannot be used on [Uno.Toolkit.UI.NavigationBar]'

Note: Even if I comment the material styles I still get the exception

I was not able to find where we wrongly set those properties in the app nor in Uno.Themes. My main suspect is https://github.com/unoplatform/uno/blob/7b4671c663cd71cac5f20565a8b48cf9f7c81606/src/Uno.UI/Controls/CommandBar/AppBarButtonRenderer.Android.cs#L55.

But anyway as this exception is raise only in debug (https://github.com/unoplatform/uno/blob/7b4671c663cd71cac5f20565a8b48cf9f7c81606/src/Uno.UI/UI/Xaml/DependencyObjectStore.cs#L642) and the effect is probably just a minor perf hit, I don't see that as a urgent issue anymore and I will transfer the issue in the uno repo.

Thanks a lot for all the details @dr1rrb !!
(cc @nickrandolph / @Xiaoy312 / @kazo0 for info)

@kazo0
Copy link
Contributor

kazo0 commented May 13, 2022

@dr1rrb So it appears that we cannot use a Converter when setting the Background of the NavigationBar. For some reason, it breaks the nested registration of the PropertyChanged callback here: https://github.com/unoplatform/uno.toolkit.ui/blob/650812afeed82afc5b330a28b50ba97625331446/src/Uno.Toolkit.UI/Controls/NavigationBar/NavigationBarRenderer.Android.cs#L151-L153

Removing the converter and just setting it directly to the resource does not cause a crash.

And yes, Only BitmapIcon can be used for AppBarButton.Icons within the NavigationBar

Also, the brushes used in the Converter have now been brought over into the Material Themes v2 package so we just need to update the packages within the ToDo app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants