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

In DemoConsole app, all tagPages are empty without controls displayed #11095

Closed
Olina-Zhang opened this issue Mar 21, 2024 · 6 comments · Fixed by #11097
Closed

In DemoConsole app, all tagPages are empty without controls displayed #11095

Olina-Zhang opened this issue Mar 21, 2024 · 6 comments · Fixed by #11097
Assignees
Labels
💥 regression-preview Regression from a preview release
Milestone

Comments

@Olina-Zhang
Copy link
Member

.NET version

.Net 9.0: Main branch of Winforms repo

Did it work in .NET Framework?

Not tested/verified

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

Yes, it is a regression issue, last week for Winforms repo testing, we can see controls in tabPages of DemoConsole test app

Issue description

In DemoConsole test app of Winforms repo, when running it, all tabPages are empty without controls displayed as following:

DemoConsoleIssue.mp4

Steps to reproduce

Test sample: DemoConsole in Winforms repo

@Olina-Zhang Olina-Zhang added untriaged The team needs to look at this issue in the next triage 💥 regression-preview Regression from a preview release labels Mar 21, 2024
@LeafShi1
Copy link
Member

LeafShi1 commented Mar 21, 2024

This issue caused by the PR #11081, PR changed null! to String.Empty at

((IDesignerHost)this).CreateComponent(componentType, string.Empty);
, but name=empty was not judged when create site
protected override ISite CreateSite(IComponent component, string? name)
{
Debug.Assert(component is not null, "Caller should have guarded against a null component");
// We need to handle the case where a component's ctor adds itself to the container. We don't want to do the
// work of creating a name, and then immediately renaming. So, DesignerHost's CreateComponent will set
// _newComponentName to the newly created name before creating the component.
if (!string.IsNullOrEmpty(_newComponentName))
{
name = _newComponentName;
_newComponentName = null;
}
INameCreationService? nameCreate = GetService(typeof(INameCreationService)) as INameCreationService;
// Fabricate a name if one wasn't provided. We try to use the name creation service, but if it is not available
// we will just use an empty string.
if (name is null)
.

@JeremyKuhne
Copy link
Member

Thanks @LeafShi1, if you want to change the condition on line 386 to be null or empty that would be great. :)

@JeremyKuhne JeremyKuhne added this to the 9.0 Preview4 milestone Mar 21, 2024
@JeremyKuhne JeremyKuhne removed the untriaged The team needs to look at this issue in the next triage label Mar 21, 2024
@Tanya-Solyanik
Copy link
Member

Tanya-Solyanik commented Mar 21, 2024

I think that the correct fix is to change nullability in Container.CreateSite method, but we don't own it. Null is a valid input and it implies that container (designer host) should try to create a new name for the component. Empty string means that component does not have a name, and we should not attempt to create one. I don't remember what scenario this is.
This issue could be fixed by reverting the change that introdcued it.

@JeremyKuhne
Copy link
Member

@Tanya-Solyanik What is the bad in creating a name for string.Empty? Can you provide any context on why not having a name is important?

@Tanya-Solyanik
Copy link
Member

@Tanya-Solyanik What is the bad in creating a name for string.Empty?

@JeremyKuhne - potentially we will be attempting to create a name multiple times in the case when the naming service is not available. Not all components are named, some inner ones are not. This is a protected virtual method, we don't know what assumption the overriders make when they call the base method, but it's reasonable to assume that anyone who implements DesignerHost or Container assumes the same behavior as in the InProc designer. We don't have sufficient test base to validate all design time scenarios in this repo, we will not see immediate failures, but this change has a potential to prevent migration.

Can you provide any context on why not having a name is important?

No, I don't remember the specific scenario

@MelonWang1
Copy link
Contributor

Verified this issue on winforms repo from main branch, it was fixed, now all tabPages show the controls.

11095.mp4

@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
💥 regression-preview Regression from a preview release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants