Skip to content

Conversation

@KodamaSakuno
Copy link
Contributor

@KodamaSakuno KodamaSakuno commented May 13, 2019

Fixes #679

The issue is that the original code forgets to check memberInfo's declaring type. When there's a property which shares a same member name of SystemParameters or SystemResourceKeyID, it'll be treated as member of SystemParameters.

The following content is a part of compiled baml of issue demo. Generated by dnSpy.

Before:

ElementStart [TypeId=0xff02, Flags=0x00]
    ContentProperty [AttributeId=0xff2b]
    LineNumberAndPosition [LineNumber=0x0000000f, LinePosition=0x0000000a]
    ElementStart [TypeId=0xffc9, Flags=0x00]
        PropertyWithExtension [AttributeId=0xfff2, Flags=0x025a, ValueId=0xfe52]
        LinePosition [LinePosition=0x00000011]
    ElementEnd
ElementEnd

After:

ElementStart [TypeId=0xff02, Flags=0x00]
    ContentProperty [AttributeId=0xff2b]
    LineNumberAndPosition [LineNumber=0x0000000f, LinePosition=0x0000000a]
    ElementStart [TypeId=0xffc9, Flags=0x00]
        TypeInfo [TypeId=0x0001, AssemblyId=0x0000, TypeFullName="XamlBugs.StaticProperties.FooClass"]
        AttributeInfo [AttributeId=0x0001, OwnerTypeId=0x0001, AttributeUsage=0x00, Name="Border"]
        PropertyWithExtension [AttributeId=0xfff2, Flags=0x025a, ValueId=0x0001]
        LinePosition [LinePosition=0x00000011]
    ElementEnd
ElementEnd

@weltkante
Copy link

Are you sure you want to fix this in the caller and don't want to put a fix in SystemResourceKey.GetBamlIdBasedOnSystemResourceKeyId? It receives the targetType as argument and then just ignores it when selecting the memberId from the enum.

@walterlv
Copy link
Contributor

I really don't like this naming XamlTypeMapper.AssemblyPF, I spend too much time to find that it indicates the PresentationFramework assembly.

@KodamaSakuno KodamaSakuno changed the title Fix bug where x:Static argument is treated as member of 'SystemParame… Fix bug where x:Static argument is treated as member of 'SystemParameters' by mistake May 13, 2019
@KodamaSakuno
Copy link
Contributor Author

@weltkante Ok, I moved it.

@dnfclas
Copy link

dnfclas commented May 13, 2019

CLA assistant check
All CLA requirements met.

if (found)
if (targetType.Assembly == XamlTypeMapper.AssemblyPF &&
targetType.FullName == "System.Windows.SystemParameters" &&
Enum.TryParse(srkField, out SystemResourceKeyID srkId))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enum.TryParse is not available on .NET 3.0 and 3.5 which support WPF and out variables require C# 7.0. Might be OK, just pointing out this is changing runtime and compilation requirements.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repository is for .NET Core 3+ only, so you can assume thats available.

Copy link
Contributor Author

@KodamaSakuno KodamaSakuno May 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's SDK code, not runtime currently. And then, as far as I'm concerned, .Net Framework 3.0 and 3.5 are both out of support at present, so this code is impossible to be a part of them. However, you can build your .Net 3.0/3.5 WPF app without this bug with the help of this SDK.
What's more, it's unnecessary to build WPF app with SDK source code referenced. So it doesn't matter to use C# 7.0+. :)

@grubioe grubioe added the PR metadata: Label to tag PRs, to facilitate with triage label May 28, 2019
Base automatically changed from master to main March 17, 2021 17:38
@ryalanms ryalanms requested a review from a team as a code owner March 17, 2021 17:38
@pchaurasia14 pchaurasia14 added the Community Contribution A label for all community Contributions label Jul 20, 2022
@ghost ghost assigned KodamaSakuno Jul 20, 2022
@dipeshmsft
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rchauhan18
Copy link
Contributor

Thanks @KodamaSakuno for your contribution.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XAML {x:Static FooClass.Border} will be compiled unexpectedly to {x:Static SystemParameters.Border}

9 participants