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

added label "ResourceKey" to trace details when resource is not found #3493

Merged

Conversation

TysonMN
Copy link
Contributor

@TysonMN TysonMN commented Sep 11, 2020

Fixes #2956

Summary of issue

When targeting .NET Framework, not finding a ResourceDictionary key named someKey causes

System.Windows.ResourceDictionary Warning: 9 : Resource not found; ResourceKey='someKey'

to be written to PresentationTraceSources.ResourceDictionarySource, but when targeting .NET Core, that message is only

System.Windows.ResourceDictionary Warning: 9 : Resource not found

Contribution

This branch contributes a fix. I want to explain both why it is a fix and why (I think) it is the correct fix.

A fix

The change adds "ResourceKey" to a string array. That array is later passed into

// Trace an event
//
// note: labels start at index 1, parameters start at index 0
//
public void Trace( TraceEventType type, int eventId, string message, string[] labels, object[] parameters )

as the labels argument. With the code currently in master, the relevant arguments for the cases in question are

  • labels = new string[] { "Resource not found" } and
  • parameters = new object[] { "someKey" }.

Confusingly, the first argument in that array is the same as the message argument, so the first entry in labels is ignored in this function. As the comment points out, note: labels start at index 1, parameters start at index 0. Then the body of this for-loop is never executed.

int i = 1, j = 0;
for( ; i < labels.Length && j < parameters.Length; i++, j++ )
{
// Append to the format string a "; {0} = '{1}'", where the index increments (e.g. the second iteration will
// produce {2} & {3}).
traceBuilder.Append("; {" + (formatIndex++).ToString() + "}='{" + (formatIndex++).ToString() + "}'" );

With my change, the relevant arguments are

  • labels = new string[] { "Resource not found", "ResourceKey" } and
  • parameters = new object[] { "someKey" }.

The body of the for-loop executes once to append ; ResourceKey='someKey' to traceBuilder.

Correct fix

No unintended consequences

A concern with any bug fix is that it doesn't only fix the bug in question but also has some unintended consequence. I don't think my change has any unintended consequences. The line I changed alters the definition of TraceResourceDictionary.ResourceNotFound. According to my searches, the only two references to that property are these two, which both lead to the code to which I linked in the previous section and the behavior I described there.

if ((fe != null && fe.IsLoaded) || (fce != null && fce.IsLoaded))
{
TraceResourceDictionary.Trace( TraceEventType.Warning,
TraceResourceDictionary.ResourceNotFound,
resourceKey );
}
else if( TraceResourceDictionary.IsEnabled )
{
TraceResourceDictionary.TraceActivityItem(
TraceResourceDictionary.ResourceNotFound,
resourceKey );
}

Comparison with .NET Framework source

I tried to analyze the same code in .NET Framework and statically verify what I know to be true via testing, which is that .NET Framework does not contain the bug in question. I didn't quite succeed though. Considering the reference source, I tracked down the same two calls to TraceResourceDictionary.ResourceNotFound, but I wasn't able to find TraceResourceDictionary in the reference source.

Generated?

I am concerned that the change I made is in a file that is in a folder called Generated. Did I edit generated code?

@TysonMN TysonMN requested a review from a team as a code owner September 11, 2020 00:56
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Sep 11, 2020
@ghost ghost requested review from fabiant3, ryalanms and SamBent September 11, 2020 00:56
@ryalanms
Copy link
Member

ryalanms commented Jan 7, 2021

Thank you for your contribution, @TysonMN.

@ryalanms ryalanms merged commit 4ecd78e into dotnet:master Jan 7, 2021
@TysonMN TysonMN deleted the fix_2956_resource_key_missing_in_warning branch January 7, 2021 14:29
@TysonMN TysonMN mentioned this pull request Apr 6, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.net core WPF System.Windows.ResourceDictionary warning miss details
2 participants