Dirkster99/AvalonDock

DocumentClosed event issue

Skaptor opened this issue · 6 comments

I found that from v4.30 and up, the content property of the closing document is null, so mi PRISM app cannot remove the region since the content is null.

private void DockingManager_DocumentClosed(object sender, DocumentClosedEventArgs e) { LayoutDocument layoutDocument = e.Document; //e.Document.content is null, breaking my region behavior using prism }

Hi,

I don't have a sample client to verify your issue :-( and so I am left to guessing at this point - but since there is:

  • only one code point that fires the DocumentClosed event in the DockingManager.ExecuteCloseCommand() method and
  • and only one change in the version window 4.30 and up

I am wondering if this change could be the cause of this issue.

Would you be able to test this with the current version 4.4 (with reference to source code project) and tell me if your issue can be fixed if we move the call to RemoveLogicalChild(uIElement); after the Event being raised (or if you see a better fix let me know also)?

internal void ExecuteCloseCommand(LayoutDocument document)
{
  if (DocumentClosing != null)
  {
    var argsClosing = new DocumentClosingEventArgs(document);
    DocumentClosing(this, argsClosing);
    if (argsClosing.Cancel) return;
  }

  if (!document.CloseDocument()) return;
  
  RemoveViewFromLogicalChild(document);
  
  DocumentClosed?.Invoke(this, new DocumentClosedEventArgs(document));
  
  if (document.Content is UIElement uIElement)
      RemoveLogicalChild(uIElement);
}

I think the issue is at the CloseInternal() function in LayoutContent.cs as the content is explicitly set to null.

internal void CloseInternal()
{
  var root = Root;
  var parentAsContainer = Parent;

  if (PreviousContainer == null)
  {
    var parentAsGroup = Parent as ILayoutGroup;
    PreviousContainer = parentAsContainer;
    PreviousContainerIndex = parentAsGroup.IndexOfChild(this);

    if (parentAsGroup is ILayoutPaneSerializable layoutPaneSerializable)
    {
      PreviousContainerId = layoutPaneSerializable.Id;
      // This parentAsGroup will be removed in the GarbageCollection below
      if (parentAsGroup.Children.Count() == 1 && parentAsGroup.Parent != null && Root.Manager != null)
      {
        Parent = Root.Manager.Layout;
        PreviousContainer = parentAsGroup.Parent;
        PreviousContainerIndex = -1;

        if (parentAsGroup.Parent is ILayoutPaneSerializable paneSerializable)
          PreviousContainerId = paneSerializable.Id;
        else
          PreviousContainerId = null;
      }
    }
  }

  parentAsContainer.RemoveChild(this);
  root?.CollectGarbage(); 
  this.Content = null;  //<--------------------------- issue is here
  OnClosed();
}

I wonder if the ExecuteCloseCommand function can be changed in order to invoke the DocumentClosed event using a new "DocumentClosedEventArgs" variable, since I need to see which content is being "closed" in order for the PRISM framework to remove it from the region?

Thank you

**EDIT: had mixed up the variable type suggestion (DocumentClosedEventArgs)

Hi @LyonJack
it turns out that we need to preserve the content in the DocumentClosed event since PRISM needs to see the content that was closed. Do you see a good fix for adjusting your leak fix #171 such that the content can still be part of the event?

@LyonJack
I have received no answer from @LyonJack on this problem so I am wondering if @Skaptor can suggest a solution that will set the content to null after having messaged the Document Closed Event? If so, please feel free to descripe the suggested change in detail or prepare a PR please, thank you :-)

@Dirkster99 Sure, I will see what I can do without breaking something, my initial guess is to invoke the event before getting rid of the content object. I will keep you posted! Thank you. :D

@Dirkster99 Hello Dirkster, I will post a PR with the modified code, I ran the unit test project, but is there a way to see if I didn't break anything else??

image