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

Some tool windows are broken if using VS.Events.WindowEvents.ActiveFrameChanged #455

Open
lordfanger opened this issue Sep 8, 2023 · 2 comments

Comments

@lordfanger
Copy link

During investigating the issue in another repository, I've identified the problem there. (or can be somewhere deeper inside VS)

The constructor ActiveFrameChangeEventArgs(IVsWindowFrame oldFrame, IVsWindowFrame newFrame) calls constructors for WindowFrame(IVsWindowFrame frame) and these in turn calls SetProperty(-3011, this) later invoking NotifyPropertyChanged("ViewHelper") in original WindowFrame possibly messing other things during Window initialization. My investigation ended here, I didn't went further.

Anyway, I was able to reproduce the bug with calling WindowFrame(IVsWindowFrame frame) myself.

VS.Events.WindowEvents.FrameIsVisibleChanged += args =>
{
    _ = new WindowFrame(args.Frame);
}

Is it possible to not create new instances and reuse the old ones when they are already WindowFrame?

@madskristensen
Copy link
Contributor

Thanks for looking into this.

What if it didn't new up a WindowFrame at all, but just returned the IVsWindowFrame. Then we can add a extension method to IVsWindowFrame that returns a WindowFrame for people to use when needed. It can return null if the WindowFrame constructor fails.

@lordfanger
Copy link
Author

Wouldn't changing signature break all current usages?

In current WindowManagerService implementation they are guaranteed to be WindowFrame or null.

WindowFrame oldFrame = varValueOld as WindowFrame;
WindowFrame newFrame = varValueNew as WindowFrame;
NotifyWindowFrameEvent(delegate(uint cookie, IVsWindowFrameEvents events)
{
    events.OnActiveFrameChanged(oldFrame, newFrame);
});

But with extension way. WindowFrame constructor should be called on main thread, maybe the async method should be created as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants