WindowsCommunityToolkit icon indicating copy to clipboard operation
WindowsCommunityToolkit copied to clipboard

ResourceNameToResourceString converter does not find the resource altough it exists.

Open Suriman opened this issue 1 year ago • 15 comments

Describe the bug

Using the converter to obtain a named resource provokes a NullReferenceException.

Regression

No response

Reproducible in sample app?

  • [ ] This bug can be reproduced in the sample app.

Steps to reproduce

Launch the attached solution, a NullReferenceException should be thrown after MainPage.InitializeComponent() is executed. If the xaml with the Converter is commented it does not throw.

The resource is defined in Strings/en/Resources.resw and the same resource name is in a property of the code-behind of MainPage.xaml and x:binded to the Text property of the TextBlock.

Expected behavior

No error should be produced and the string resource should be set to the TextBlock.

Screenshots

No response

Windows Build Number

  • [ ] Windows 10 1809 (Build 17763)
  • [ ] Windows 10 1903 (Build 18362)
  • [ ] Windows 10 1909 (Build 18363)
  • [ ] Windows 10 2004 (Build 19041)
  • [ ] Windows 10 20H2 (Build 19042)
  • [ ] Windows 10 21H1 (Build 19043)
  • [X] Windows 11 21H2 (Build 22000)
  • [ ] Other (specify)

Other Windows Build number

No response

App minimum and target SDK version

  • [ ] Windows 10, version 1809 (Build 17763)
  • [X] Windows 10, version 1903 (Build 18362)
  • [ ] Windows 10, version 1909 (Build 18363)
  • [X] Windows 10, version 2004 (Build 19041)
  • [ ] Other (specify)

Other SDK version

No response

Visual Studio Version

2022

Visual Studio Build Number

Version 17.3.0 Preview 2.0

Device form factor

Desktop

Nuget packages

Microsoft.WindowsAppSDK 1.1.0 CommunityToolkit.WinUI.UI 7.1.2

Additional context

ResourceNameToResourceStringConverterIssueWinUI.zip

Help us help you

No.

Suriman avatar Jul 18 '22 14:07 Suriman

Hello Suriman, 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 Jul 18 '22 14:07 ghost

I've verified that the bug is there in the provided solution, and I've triaged this so we can begin work on it.

Additional information for the person who fixes this, this is the place where the nullref happens: image

Arlodotexe avatar Jul 22 '22 17:07 Arlodotexe

May be related? https://github.com/microsoft/WindowsAppSDK/issues/1674

michael-hawker avatar Aug 23 '22 16:08 michael-hawker

@Suriman this is not an issue with the Toolkit, your key needs to be Resources/ConverterResource to look up the resource you want as part of the path to the resource file.

https://docs.microsoft.com/en-us/windows/apps/windows-app-sdk/mrtcore/mrtcore-overview

As seen in the raw API sample here as well: https://github.com/microsoft/WindowsAppSDK-Samples/tree/main/Samples/ResourceManagement/cs-winui/winui_desktop_packaged_app

@azchohfi I noticed in the string extension we do look for Resources sub map as part of the logic here:

https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/080bdb99e6810eb2157ad635f00e64a64acabb50/CommunityToolkit.WinUI/Extensions/StringExtensions.cs#L54

But that's still dependent on the developer leaving the default file name of Resources.resw right? We should always leave it up to the developer to specify the path, no?

michael-hawker avatar Aug 23 '22 17:08 michael-hawker

I believe this is the "Resources" xml tag inside the file, not the file name itself. I'm not sure how the resource manager works on regards to bundling things in the final resource dictionary.

azchohfi avatar Aug 23 '22 17:08 azchohfi

@LalithaNadimpalli since you have the project setup, mind quickly testing what happens if you rename the Resources.resw file?

michael-hawker avatar Aug 23 '22 18:08 michael-hawker

Hello world is not displayed just shows an " A resource loaded via converter" msg on the output window

LalithaNadimpalli avatar Aug 23 '22 19:08 LalithaNadimpalli

@LalithaNadimpalli could you elaborate on that compared to the normal behavior? Trying to remember what the sample did by default, so I've missed the context somewhere. Thanks! (And what outputs the message in the output window, is it part of the repro code?)

michael-hawker avatar Aug 24 '22 06:08 michael-hawker

The string that the user stored looks like the screenshot attached. String When we change the name from Resources.resw to let's say Resource.rsew and pass it in xml.cs file public string TestResourceKey = "Resource/ConverterResource"; the output window looks like Screenshot 2022-08-23 120458

LalithaNadimpalli avatar Aug 24 '22 15:08 LalithaNadimpalli

Ah, so it does look like the path passed into the converter matches the file name, e.g. if the file was named "Foo.resw" you'd need to do "Foo/ConverterResource".

michael-hawker avatar Aug 24 '22 19:08 michael-hawker

Yes, I did the same if eg: the file is abc.resw and then we pass the TestResourceKey=abc/ConverterResource it shows the output as Screenshot 2022-08-23 120458 but if the file name is Resources.resw and then pass TestResourceKey=Resources/ConverterResource it shows the output Screenshot 2022-08-24 140240

LalithaNadimpalli avatar Aug 24 '22 21:08 LalithaNadimpalli

Looks like for x:Uid and the XAML PRI system it kind of assumes there'll be a Resources.resw/pri file, see docs here:

In some cases you'll be using a resource path rather than built-in functionality of the package resource index (PRI) system. Any string used as an x:Uid value defines a resource path that begins with ms-resource:///Resources/ and includes the x:Uid string. The path is completed by the names of the properties you specify in a resources file or are otherwise targeting.

And here:

A package typically contains a single PRI file per language, named resources.pri. The resources.pri file at the root of each package is automatically loaded when the ResourceManager is instantiated.

I think you can grab a subtree with the resource key as a path, but maybe this is something we just need better tests for in 8.0 when it'll be easier for us to test and validate the behavior is the same across UWP and WinUI 3? @azchohfi thoughts?

michael-hawker avatar Aug 25 '22 16:08 michael-hawker

I thought that the compilation process bundled everything into a resources.pri file, and that is why it had to start with "Resources/*". Maybe @RealTommyKlein can clarify.

azchohfi avatar Aug 26 '22 17:08 azchohfi

Hi. I'm a colleague of @Suriman. The same sample project using UWP works without having to prefix the key with 'Resources'. Furthermore, it does not provoke the NullReferenceException if it does not find the key, which happens when you do public string TestResourceKey = "Resources/ConverterResource".

ADD-David-Antolin avatar Sep 06 '22 06:09 ADD-David-Antolin

I thought that the compilation process bundled everything into a resources.pri file, and that is why it had to start with "Resources/*". Maybe @RealTommyKlein can clarify.

We do bundle all app resources into a resources.pri by default. There's also a separate resources.pri that ships with the Windows App SDK framework package itself which is handled/loaded separately by the WinUI 3 framework, containing resources like generic.xaml and builtin theme/static resources.

I think the notion of "PRI file name = MRT resource map name" is correct, so to reference a resource Foo in MyPriFile.pri you would need to use MyPriFile/Foo, and because our default resources name is resources.pri every resource usually starts with Resources/*. @evelynwu-msft to check my understanding.

RealTommyKlein avatar Sep 06 '22 22:09 RealTommyKlein

So, these are the two bits of code we're comparing from UWP:

        private readonly ResourceLoader _resourceLoader = ResourceLoader.GetForViewIndependentUse();

        public object Convert(object value, Type targetType, object parameter, string language)
        {
            if (value == null)
            {
                return string.Empty;
            }

            return _resourceLoader.GetString(value.ToString());
        }

And Windows App SDK:

        private readonly ResourceManager _resourceManager = new ResourceManager();

        public object Convert(object value, Type targetType, object parameter, string language)
        {
            if (value == null)
            {
                return string.Empty;
            }

            return _resourceManager.MainResourceMap.TryGetValue(value.ToString()).ValueAsString;
        }

It would seem that they're behaving differently.

Our extensions here are also slightly different as well between the two frameworks:

https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/b2ea7ba59ce3c869e155bbe5fcd6b608b59bc32a/Microsoft.Toolkit.Uwp/Extensions/StringExtensions.cs#L56-L67

https://github.com/CommunityToolkit/WindowsCommunityToolkit/blob/080bdb99e6810eb2157ad635f00e64a64acabb50/CommunityToolkit.WinUI/Extensions/StringExtensions.cs#L48-L62

I think the best solution is for us to wait until our next major release where we'll be better able to right tests that run against both platforms and we can align the code better to provide the same results. There's at least a workaround in place at the moment.

michael-hawker avatar Oct 17 '22 17:10 michael-hawker