[SamplesApp] `NullReferenceException` setting cursors

spouliot commented 6 months ago

Current behavior

Debug Output shows

Exception thrown: 'System.NullReferenceException' in SamplesApp.Skia.dll
fail: Microsoft.UI.Xaml.UIElement[0]
      Failed to raise 'TappedEvent': System.NullReferenceException: Object reference not set to an instance of an object.
         at SamplesApp.Wasm.Windows_UI_Core.SetCursor.ResetTapped(Object sender, TappedRoutedEventArgs e) in /Users/poupou/git/external/uno/uno/src/SamplesApp/UITests.Shared/Windows_UI_Core/SetCursor.xaml.cs:line 63
         at SamplesApp.Wasm.Windows_UI_Core.SetCursor.<>c__DisplayClass11_0.<InitializeComponent>b__7(Object ResetTapped_sender, TappedRoutedEventArgs ResetTapped_e) in /Users/poupou/git/external/uno/uno/src/SamplesApp/SamplesApp.Skia/Uno.UI.SourceGenerators/Uno.UI.SourceGenerators.XamlGenerator.XamlCodeGenerator/SetCursor_17f3bfdba1899721a939a7e34569b374.cs:line 108
         at Microsoft.UI.Xaml.UIElement.InvokeHandler(Object handler, RoutedEventArgs args) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/UIElement.RoutedEvents.cs:line 977
         at Microsoft.UI.Xaml.UIElement.RaiseEvent(RoutedEvent routedEvent, RoutedEventArgs args, BubblingContext ctx) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/UIElement.RoutedEvents.cs:line 697
         at Microsoft.UI.Xaml.UIElement.SafeRaiseEvent(RoutedEvent routedEvent, RoutedEventArgs args, BubblingContext ctx) in /Users/poupou/git/external/uno/uno/src/Uno.UI/UI/Xaml/UIElement.RoutedEvents.cs:line 646

At line 55 of SetCursor.xaml.cs the value of Microsoft.UI.Xaml.Window.Current.CoreWindow is null which throws the `

Microsoft.UI.Xaml.Window.Current.CoreWindow.PointerCursor = new global::Windows.UI.Core.CoreCursor((global::Windows.UI.Core.CoreCursorType)Box.SelectedItem, 0);

Expected behavior

No exception.

How to reproduce it (as minimally and precisely as possible)

Build and execute SampleApp.
Select the SetCursor page
Select a cursor (combobox)

The cursor does not change (to the selected cursor name) and an exception is logged (see Debug Output).

Workaround

None.

Works on UWP/WinUI

None

Environment

No response

NuGet package version(s)

No response

Affected platforms

Skia (GTK on Linux/macOS/Windows)

IDE

No response

IDE version

No response

Relevant plugins

No response

Anything else we need to know?

No response

ramezgerges commented 6 months ago

This is a problem on WinUI, which deprecated Window.Current. It will work fine on the uwp tree. To set the cursor on WinUi (and uno), use ProtectedCursor, which I implemented recently.

MartinZikmund commented 1 month ago

The suggestion from @ramezgerges is correct solution. CoreWindow will always be null in Uno.WinUI

spouliot commented 1 month ago

The issue was more a reminder to somehow [1] fix SampleApp than how to implement the new code for cursors (which already exists) 😄

[1] maybe just tell which the other test that covers the new code ? not sure how it's being handled for similar cases...

ramezgerges commented 1 month ago

@spouliot there is a ProtectedCursor sample if I remember correctly.

MartinZikmund commented 1 month ago

Ah sorry, didn't realize ProtectedCursor might not be working on macOS yet, reopened for now then

spouliot commented 1 month ago

@ramezgerges yes, I know there is :)

@MartinZikmund no, it's implemented/working on macOS :)

However anyone trying SampleApp might not know which test to use for cursors (I did not the first time).

Having the old test fail makes it look like cursor support is broken or incomplete.

Maybe that old test should be #if out when building for WinUI ?

unoplatform / uno