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

ResourceCodeDomSerializer fails to assign distinct number suffixes to resources when serializing collection values #12595

Open
codereader opened this issue Dec 5, 2024 · 6 comments
Labels
area-DesignerSupport 💥 regression-framework Regression from .NET Framework untriaged The team needs to look at this issue in the next triage

Comments

@codereader
Copy link

codereader commented Dec 5, 2024

.NET version

8.0.201 (the affected code is still there in .NET 9)

Did it work in .NET Framework?

Yes

Did it work in any of the earlier releases of .NET Core or .NET 5+?

No response

Issue description

The code in ResourceCodeDomSerializer.SerializationResourceManager.SetValue() (around code lines 714) is failing when assigning sequential number suffixes to resource names:

While I cannot provide an quick-and-easy project to reproduce this, I'd like to suggest reading the code instead:

....
// Now find an unused name
string resourceName = nameBase;

// Only append the number when appendCount is set or if there is already a count.
int count = 0;
if (appendCount || _nameTable.TryGetValue(nameBase, out count))
{
    count++;
    resourceName = $"{nameBase}{count}";
}

// Now that we have a name, write out the resource.
SetValue(manager, resourceName, value, forceInvariant, shouldSerializeInvariant, ensureInvariant, applyingCachedResources);
_nameTable[resourceName] = count; // <<--- bug here, _nameTable[nameBase] should be assigned instead
....

The code in .NET 4.8 assigned numbers starting from 1..N (even if only one value was in the _nameTable, so from the comment in the code I assume it has been changed to avoid assigning a number to the very first occurrence.

The bug occurs when generating number suffixes for the same nameBase value more than twice - it will end up using the same number 1 over and over. You'll end up with a _nameTable looking like this:

  • "name" => 0
  • "name1" => 1

"name1" will be overwritten every time after the second.

Steps to reproduce

The bug is recurring in our software when copy/pasting WinForms controls in design mode. We are maintaining a Window Editor application which allows to copy/paste controls. When serializing the controls, the CodeDomSerializer will end up generating code like this (note the resource names, which are always using the number 1 as suffix.

CustomTextField.CustomProperty.AddRange(new CustomAssembly.Controls.Collections.CollectionItem[] {
			((CustomAssembly.Controls.Collections.CollectionItem)(resources.GetObject("CustomTextField.CustomProperty"))),
			((CustomAssembly.Controls.Collections.CollectionItem)(resources.GetObject("CustomTextField.CustomProperty1"))),
			((CustomAssembly.Controls.Collections.CollectionItem)(resources.GetObject("CustomTextField.CustomProperty1"))),
			((CustomAssembly.Controls.Collections.CollectionItem)(resources.GetObject("CustomTextField.CustomProperty1")))});
@codereader codereader added the untriaged The team needs to look at this issue in the next triage label Dec 5, 2024
@merriemcgaw
Copy link
Member

merriemcgaw commented Dec 11, 2024

@Olina-Zhang can you work with this customer to build up some sort of repro scenario?

@Olina-Zhang
Copy link
Member

@codereader I tried to repro your issue, .NET 9 and .NET framework apps have the same result with a sample app:
WinFormsApp18.zip
Image

Can you please help to see if is that your issue?

@codereader
Copy link
Author

Hello, thanks for taking the time to look into this :)

The ZIP file doesn't quite reproduce the problem. It differs by two points:

First, the SetValue code in your screenshot already features the corrected way of memorizing the count:

_nameTable[nameBase] = count;.

In the current .NET 9 serializer, the line reads like this:

_nameTable[resourceName] = count;

See

Second, in my production scenario, the appendCount parameter value is false, as it is during the serialization of elements of a collection property. The nameBase value passed to the SetValue method is identical for all N collection elements.

I attached a project, where I adjusted the .NET 9 version to reproduce the problem. Clicking the button should yield the following result:

nameBase btnControl => generated ResourceName = btnControl1
nameBase btnControl1.SomePropertyName => generated ResourceName = btnControl1.SomePropertyName
nameBase btnControl1.SomePropertyName => generated ResourceName = btnControl1.SomePropertyName1
nameBase btnControl1.SomePropertyName => generated ResourceName = btnControl1.SomePropertyName1
nameBase btnControl2.SomePropertyName => generated ResourceName = btnControl2.SomePropertyName
nameBase btnControl2.SomePropertyName => generated ResourceName = btnControl2.SomePropertyName1
nameBase btnControl2.SomePropertyName => generated ResourceName = btnControl2.SomePropertyName1
nameBase btnControl3.SomePropertyName => generated ResourceName = btnControl3.SomePropertyName
nameBase btnControl3.SomePropertyName => generated ResourceName = btnControl3.SomePropertyName1
nameBase btnControl3.SomePropertyName => generated ResourceName = btnControl3.SomePropertyName1

Note the same resource name being generated for the same nameBase values (starting from the third invocation).

@Olina-Zhang
Copy link
Member

@codereader It seems you forgot to upload your project, can you please provide it? I will use it for testing again. Thanks!

@Olina-Zhang
Copy link
Member

I created another project to repro this issue, if use _nameTable[resourceName] = count;, it will generate unexpected resource name starting from the third invocation.
Image
After changing with using _nameTable[nameBase] = count;, all generated resource name are non-unique
Image

Please see following video:

ResourceIssue.mp4

Repro app: WinFormsApp19.zip

@codereader
Copy link
Author

Yikes, apologies for this classic mistake. I attached the project now.

WinFormsApp18_repro.zip

I checked your video, yes, this looks more like the behaviour I am seeing. If you run the .NET 9 project in my ZIP, you should see very similar output. Thanks for taking the extra time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-DesignerSupport 💥 regression-framework Regression from .NET Framework untriaged The team needs to look at this issue in the next triage
Projects
None yet
Development

No branches or pull requests

4 participants