`new ResourceLoader()` Returns an Unusable ResourceLoader

robloo commented 3 years ago

Current behavior

Some existing UWP code was using var resources = new ResourceLoader() which works fine albeit is very bad practice. In the Uno Platform, the ResourceLoader returned like this is not usable for looking up resources.

Expected behavior

new ResourceLoader() should return a usable resource loader likely equivalent to ResourceLoader.GetForCurrentView(). This is only to be source compatible matching the likely undocumented behavior of UWP.

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

Workaround

Use ResourceLoader.GetForCurrentView() or ResourceLoader.GetForViewIndependentUse() which is best practice and follows the docs.

Environment

Nuget Package:

[x] Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia
[ ] Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia
[ ] Uno.SourceGenerationTasks
[ ] Uno.UI.RemoteControl / Uno.WinUI.RemoteControl
[ ] Other:

Nuget Package Version(s): 3.6.6

Affected platform(s):

[ ] iOS
[x] Android
[ ] WebAssembly
[ ] WebAssembly renderers for Xamarin.Forms
[ ] macOS
[ ] Skia
- [ ] WPF
- [ ] GTK (Linux)
- [ ] Tizen
[ ] Windows
[ ] Build tasks
[ ] Solution Templates

IDE:

[ ] Visual Studio 2017 (version: )
[x] Visual Studio 2019 (version: )
[ ] Visual Studio for Mac (version: )
[ ] Rider Windows (version: )
[ ] Rider macOS (version: )
[ ] Visual Studio Code (version: )

Relevant plugins:

[ ] Resharper (version: )

Anything else we need to know?

In the Uno Docs https://platform.uno/docs/articles/guides/localization.html#step-by-steps .GetForViewIndependentUse() is used. I wonder why .GetForCurrentView() isn't preferred? Perhaps this should be changed.

aesteves900 commented 2 years ago

Hello, UWP and Skia still working when using ResourceLoader(Windows.ApplicationModel.Resources.ResourceLoader) as a static reference or instance. I'm using the Uno.Samples app and work perfect. All are the automation tests work as well. We can use new ResourceLoader().GetString("") instead of ResourceLoader.GetForCurrentView() with no problems, both has the same effect.

robloo commented 2 years ago

@aesteves900 The issue was a new instance of ResourceLoader constructed by new ResourceLoader() does not work. There are many work arounds, true, but this doesn't match UWP behavior.

francoistanguay commented 2 years ago

@robloo is right, we need to try to match the UWP behavior and support default ctor.

aesteves900 commented 2 years ago

@robloo and @francoistanguay , I follow this path, always I start mimicking UWP, I keep working on it.

aesteves900 commented 2 years ago

@robloo, I did one more test on Android that's return the valid resource all the time when I'm creating a new ResourceLoader()

AndroidResourceTest

Still working, I'm missing something?

robloo commented 2 years ago

Perhaps an unknown change solved this issue. I will retest on my end when I get a chance.

robloo commented 2 years ago

@aesteves900 Thanks for rechecking this. I have tested with latest Uno 4.1.8 and I works for most views using 'new ResourceLoader()' -- this may be an improvement. However, it does not work it seems when called from App.cs using code below (note this is my guess of a repro as the real issue is ContentDialogs are showing empty, but this seems to be the code path to get to that point).

In app.cs:

    public sealed partial class App : Application
    {
        private ResourceLoader resources = null; // Must be set later

        protected override void OnLaunched(LaunchActivatedEventArgs e)
        {
            // Setup the resource loader (cannot be done in the app constructor)
            this.resources = new ResourceLoader();
            return;
        }
    }

It is known that ResourceLoader can't be used in the App constructor -- so it is set in the OnLaunched method. This works on UWP but doesn't appear to work for Uno.

This may be something to just document and ignore for now. It is minor since it's working for other views now (or perhaps always did).

unoplatform / uno