Fixing validation adorner for not yet visible controls

[batzen]

This is a fix for #1166

[batzen]

@dotnet/wpf-developers any news on this one?

[dipeshmsft]

Hey @batzen, we ran the test pass on this PR and are ready to take this PR. Can you please resolve the conflicts in this PR? Thanks a lot for this contribution.

[batzen]

@dipeshmsft There were no conflicts.
I rebased just to be sure.

After pulling in all the changes from main i can't compile anymore as i get the error

C:\DEV\OSS_Own\dotnet\wpf\eng\WpfArcadeSdk\tools\Wpf.Cpp.targets(552,5): error : 'WriteOnlyWhenDifferent' attribute on
"C:\DEV\OSS_Own\dotnet\wpf\artifacts\obj\uce\Win32\Debug\CppSupport\CppSupport_uce.csproj" takes effect only when 'Over
write="true"'. [C:\DEV\OSS_Own\dotnet\wpf\src\Microsoft.DotNet.Wpf\src\WpfGfx\core\uce\uce.vcxproj]
    0 Warning(s)
    179 Error(s)

Any hints on how to get it to compile?

[miloush]

Can the element not become IsVisible = true before the loading finishes? Because then the repeated attempt would fail and you would not get the Loaded attempt which you get now otherwise.

[batzen]

I don't know how a not loaded control should be visible.

[dipeshmsft]

@miloush were you able to get any instances where the above condition will hold true? Or if you can suggest a way to check that?

[miloush]

How about this:

using System;
using System.Collections;
using System.ComponentModel;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;

class AlwaysError : INotifyDataErrorInfo
{
    public bool HasErrors => true;
    public event EventHandler<DataErrorsChangedEventArgs> ErrorsChanged;
    public IEnumerable GetErrors(string propertyName) => new[] { "Error" };
}

class Program
{
    [STAThread]
    static void Main(string[] args)
    {
        Window w = new();
        TextBox t = new() { Margin = new Thickness(50) };
        BindingOperations.SetBinding(t, TextBox.TextProperty, new Binding("HasErrors") { Source = new AlwaysError(), Mode=BindingMode.OneWay });
        t.Loaded += delegate
        {
            if (t.Parent is not AdornerDecorator)
            {
                AdornerDecorator ad = new();
                w.Content = ad;
                ad.Child = t;
            }
        };

        w.Template = new ControlTemplate(typeof(Window)) { VisualTree = new FrameworkElementFactory(typeof(ContentPresenter)) };
        w.Content = t;
        new Application().Run(w);
    }
}

If I am right this will get a decorator currently but will not after this PR.

EDIT: it would if Window did not come with a decorator by default.
EDIT2: ...so just use a Window template without decorator, amended.

[miloush]

Also what if the control never gets visible? Aren't we leaking references by subscribing to IsVisibleChanged?

[batzen]

We won't leak as the handler is static and not an instance member, but i will also test that.

Will have a look at the sample you posted.

[miloush]

RenderTargetBitmap might be another scenario that could be affected. EDIT: RTB is broken anyway, I can't get the adorner show up easily.

Since the code already protects against re-entrancy, why don't we just keep the current codepath and add your visibility check?

[miloush]

The basic premise is that IsVisible is set to true for all controls before the Loaded events are fired (and the controls first displayed) and that is still before the dispatcher gets to the Loaded priority operations. That is a lot of opportunities in between for people to mess up with the tree.

[batzen]

Good points.
Will have a look at the workaround code i am using in our products again.

It's been nearly four years since i wrote the PR...

[batzen]

@dipeshmsft @miloush I made changes that should prevent the code from introducing any regressions.
I also confirmed that the changes don't lead to leaks if an element never gets visible.

[dipeshmsft]

@dipeshmsft There were no conflicts. I rebased just to be sure.

After pulling in all the changes from main i can't compile anymore as i get the error

C:\DEV\OSS_Own\dotnet\wpf\eng\WpfArcadeSdk\tools\Wpf.Cpp.targets(552,5): error : 'WriteOnlyWhenDifferent' attribute on
"C:\DEV\OSS_Own\dotnet\wpf\artifacts\obj\uce\Win32\Debug\CppSupport\CppSupport_uce.csproj" takes effect only when 'Over
write="true"'. [C:\DEV\OSS_Own\dotnet\wpf\src\Microsoft.DotNet.Wpf\src\WpfGfx\core\uce\uce.vcxproj]
    0 Warning(s)
    179 Error(s)

Any hints on how to get it to compile?

@batzen I see, I will put up a workaround that I had for this up after which you will be able to build it locally.

[dipeshmsft]

@batzen I have made the necessary changes in this PR #7678, you can try it once now.

[dipeshmsft]

@batzen, thanks for your contribution. Once, the right area of the test is open-sourced, I think we should add the tests for this fix.