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

Hiding and showing anchorable in document's pane throws an exception #71

Open
Rasenshurikenbum opened this issue Sep 5, 2019 · 25 comments
Labels

Comments

@Rasenshurikenbum
Copy link

Rasenshurikenbum commented Sep 5, 2019

Steps to reproduce:

  1. Download this sample project: https://www.codeproject.com/KB/WPF/483507/Version_03_Edi_RecentFilesTW.zip
  2. Update AvalonDock version to 3.5.12.
  3. Drag 'Recent Files' anchorable window and drop it to the document's pane.
  4. Drag 'File Stats' anchorable window and drop it to the document's pane.
  5. Close 'File Stats' with X.
  6. Close 'Recent Files' with X.
  7. Choose 'Tools' from the menu and click 'RecentFiles'.
  8. It throws:
    System.InvalidCastException: 'Unable to cast object of type 'Xceed.Wpf.AvalonDock.Layout.LayoutAnchorable' to type 'Xceed.Wpf.AvalonDock.Layout.ILayoutPanelElement'.'

Other exception:

  1. Repeat step 1-6.
  2. Choose 'Tools' from the menu and click 'Properties'.
  3. Repeat step 2.
  4. It throws:
    System.ArgumentOutOfRangeException: 'Index must be within the bounds of the List. Parameter name: index'
@Rasenshurikenbum Rasenshurikenbum changed the title Closing anchorable in document's pane prevents it from showing again or throws an exception Closing anchorable in document's pane throws an exception Sep 5, 2019
@Rasenshurikenbum Rasenshurikenbum changed the title Closing anchorable in document's pane throws an exception Hiding and showing anchorable in document's pane throws an exception Sep 5, 2019
@Dirkster99
Copy link
Owner

Hi, would you please attach the updated version that works with 3.5.12 to verify your problem using your test client?

Thank you.

@Rasenshurikenbum
Copy link
Author

Rasenshurikenbum commented Sep 5, 2019

If you meant to upload my test project then there it is.
Edi_AvalonDock_v3.5.12.zip

@Dirkster99
Copy link
Owner

Dirkster99 commented Sep 7, 2019

hmmm, this is strange. I cannot verify your prolem although your workflow description is detailled and easy to follow. Is it possible that you used an older than 3.5.12 version and did not update correctly to the recent version (because there was a recent fix in 3.5.11 about this)?

@Rasenshurikenbum
Copy link
Author

Rasenshurikenbum commented Sep 7, 2019

I tried both overwriting old version and installing a fresh one. Every time I get the same results. I also found out that the first exception occurs in all versions from 3.5.4 up. In version 3.5.3 no exception is thrown but instead it doesn't let the closed window to be shown again (choosing the proper view from menu does nothing).
What does it mean you cannot verify my problem? Have you tried the project I sent before?

@Dirkster99
Copy link
Owner

Dirkster99 commented Sep 7, 2019

What does it mean you cannot verify my problem?

I have tested the application that you attached and it works for me without any exception being thrown. Its really strange - not sure how to fix a problem I cannot verify on my computer :-(

Would you please give me the full Statcktrace of the exception that you see? It should normally look like the Stacktrace in this Issue. Thanx

@Rasenshurikenbum
Copy link
Author

Rasenshurikenbum commented Sep 7, 2019

Here's the stack trace:

System.InvalidCastException
HResult=0x80004002
Message=Unable to cast object of type 'Xceed.Wpf.AvalonDock.Layout.LayoutAnchorable' to type 'Xceed.Wpf.AvalonDock.Layout.ILayoutPanelElement'.
Source=Xceed.Wpf.AvalonDock
StackTrace:
at Xceed.Wpf.AvalonDock.Layout.LayoutGroup1.InsertChildAt(Int32 index, ILayoutElement element) at Xceed.Wpf.AvalonDock.Layout.LayoutAnchorable.Show() at Xceed.Wpf.AvalonDock.Controls.LayoutAnchorableItem.OnVisibilityChanged() at Xceed.Wpf.AvalonDock.Controls.LayoutItem.<>c.<.cctor>b__13_1(DependencyObject s, DependencyPropertyChangedEventArgs e) at System.Windows.PropertyChangedCallback.Invoke(DependencyObject d, DependencyPropertyChangedEventArgs e) at System.Windows.DependencyObject.OnPropertyChanged(DependencyPropertyChangedEventArgs e) at System.Windows.FrameworkElement.OnPropertyChanged(DependencyPropertyChangedEventArgs e) at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args) at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType) at System.Windows.DependencyObject.InvalidateProperty(DependencyProperty dp, Boolean preserveCurrentValue) at System.Windows.Data.BindingExpressionBase.Invalidate(Boolean isASubPropertyChange) at System.Windows.Data.BindingExpression.TransferValue(Object newValue, Boolean isASubPropertyChange) at MS.Internal.Data.ClrBindingWorker.NewValueAvailable(Boolean dependencySourcesChanged, Boolean initialValue, Boolean isASubPropertyChange) at MS.Internal.Data.PropertyPathWorker.UpdateSourceValueState(Int32 k, ICollectionView collectionView, Object newValue, Boolean isASubPropertyChange) at MS.Internal.Data.ClrBindingWorker.OnSourcePropertyChanged(Object o, String propName) at System.Windows.WeakEventManager.ListenerList1.DeliverEvent(Object sender, EventArgs e, Type managerType)
at System.ComponentModel.PropertyChangedEventManager.OnPropertyChanged(Object sender, PropertyChangedEventArgs args)
at Edi.ViewModel.Base.ViewModelBase.RaisePropertyChanged(String propertyName) in C:\Users\Krzysztof\Desktop\Edi_AvalonDock_v3.5.12\Edi\ViewModel\Base\ViewModelBase.cs:line 13
at Edi.ViewModel.Base.ToolViewModel.set_IsVisible(Boolean value) in C:\Users\Krzysztof\Desktop\Edi_AvalonDock_v3.5.12\Edi\ViewModel\Base\ToolViewModel.cs:line 34

And here is the second exception's stack trace:

System.ArgumentOutOfRangeException
HResult=0x80131502
Message=Index must be within the bounds of the List.
Parameter name: index
Source=mscorlib
StackTrace:
at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource)
at System.Collections.ObjectModel.Collection1.Insert(Int32 index, T item) at Xceed.Wpf.AvalonDock.Layout.LayoutAnchorable.Show() at Xceed.Wpf.AvalonDock.Controls.LayoutAnchorableItem.OnVisibilityChanged() at Xceed.Wpf.AvalonDock.Controls.LayoutItem.<>c.<.cctor>b__13_1(DependencyObject s, DependencyPropertyChangedEventArgs e) at System.Windows.PropertyChangedCallback.Invoke(DependencyObject d, DependencyPropertyChangedEventArgs e) at System.Windows.DependencyObject.OnPropertyChanged(DependencyPropertyChangedEventArgs e) at System.Windows.FrameworkElement.OnPropertyChanged(DependencyPropertyChangedEventArgs e) at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args) at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType) at System.Windows.DependencyObject.InvalidateProperty(DependencyProperty dp, Boolean preserveCurrentValue) at System.Windows.Data.BindingExpressionBase.Invalidate(Boolean isASubPropertyChange) at System.Windows.Data.BindingExpression.TransferValue(Object newValue, Boolean isASubPropertyChange) at MS.Internal.Data.ClrBindingWorker.NewValueAvailable(Boolean dependencySourcesChanged, Boolean initialValue, Boolean isASubPropertyChange) at MS.Internal.Data.PropertyPathWorker.UpdateSourceValueState(Int32 k, ICollectionView collectionView, Object newValue, Boolean isASubPropertyChange) at MS.Internal.Data.ClrBindingWorker.OnSourcePropertyChanged(Object o, String propName) at System.Windows.WeakEventManager.ListenerList1.DeliverEvent(Object sender, EventArgs e, Type managerType)
at System.ComponentModel.PropertyChangedEventManager.OnPropertyChanged(Object sender, PropertyChangedEventArgs args)
at Edi.ViewModel.Base.ViewModelBase.RaisePropertyChanged(String propertyName) in C:\Users\Krzysztof\Desktop\Edi_AvalonDock_v3.5.12\Edi\ViewModel\Base\ViewModelBase.cs:line 13
at Edi.ViewModel.Base.ToolViewModel.set_IsVisible(Boolean value) in C:\Users\Krzysztof\Desktop\Edi_AvalonDock_v3.5.12\Edi\ViewModel\Base\ToolViewModel.cs:line 34

@Dirkster99
Copy link
Owner

OK, I tried it again and I am able to also get the exception. It is important to close the tool window with X because I previously made the mistake of closing it via Tools>... menu entry and it did work in this instace. Now I can look for a fix ... thank you for you patience

@Rasenshurikenbum
Copy link
Author

Awesome, hope it's not something difficult to fix.

@Dirkster99
Copy link
Owner

hmmm, I've been looking at this for a while but cannot seem to determine why the exception occurs only if I close the document with 'X' and I also noticed that it only occurs for the first tool window that is dragged into the document pane (?) but the workflow actually works for the 2nd tool window:

1 Using the attached sample project (with LayoutInitializer disabled for testing for above URL):
2 Update AvalonDock version to 4.0
3 Drag 'Recent Files' anchorable window and drop it to the document's pane.
4 Drag 'File Stats' anchorable window and drop it to the document's pane.
5 Close 'File Stats' with X.
6 Close 'Recent Files' with X.
7 Choose 'Tools' from the menu and click 'RecentFiles'. Throws System.InvalidCastException: 'Unable to cast object of type 'AvalonDock.Layout.LayoutAnchorable' to type AvalonDock.Layout.ILayoutPanelElement'.' is thrown in LayoutAnchorable.Show() at about line 449 where this is to be inserted in the previousContainerAsLayoutGroup

Repeat steps 1-4
5 Close 'Recent Files' with X.
6 Close 'File Stats' with X.
7 Choose 'Tools' from the menu and click 'RecentFiles' (no Exception is thrown - tool window re-appears in document pane after switching the property)
8 Choose 'Tools' from the menu and click 'Properties' Throws the above exception but on a different tool window

I also noticed that the PreviousContainerIndex can be -1 in the InsertAt statement so the code below could be a minor improvement in LayoutAnchorable.show() at about line 447 (but its not fixing the problem reported here):

var previousContainerAsLayoutGroup = PreviousContainer as ILayoutGroup;
if (PreviousContainerIndex < previousContainerAsLayoutGroup.ChildrenCount)
    previousContainerAsLayoutGroup.InsertChildAt((PreviousContainerIndex < 0 ? 0 : PreviousContainerIndex), this);
else
    previousContainerAsLayoutGroup.InsertChildAt(previousContainerAsLayoutGroup.ChildrenCount, this);

Whats really bothering me (because I cannot explain it) is why the same code works on one of the tool windows but not for the other even though conditions seem to be the same in both instances (inserting a LayoutAnchorable into a ILayoutGroup) but one of them ends in an exception(!)...

@scdmitryvodich Do you see what I am missing? Why is this exception one of the tool windows and not on both?

Version_03_Edi_RecentFilesTW.zip

@Muhahe
Copy link
Contributor

Muhahe commented Mar 4, 2020

@Dirkster99 i noticed this exception too, and trying to figure out some workaround. But im curious why "close" button on LayoutAnchorable works like close instead of hide. Afaik Anchorables in anchorablePanel has "hide" button, so is there some reason that Anchorable will close?

If no, then i think that replacing Close behavior for Hide behavior for LayoutAnchorables in docoment panel should work.

Also i noticed that closing and reopening of layout anchorable loses layout position. But hiding and unhiding preserves locations. So its another plus for me.

@Muhahe
Copy link
Contributor

Muhahe commented Mar 4, 2020

@Dirkster99 @Rasenshurikenbum
so as i mentioned in post above, i tried to replace CloseCommand with HideCommand for LayoutAnchorableItem.cs row 198

protected override void Close()
{
	if (_anchorable.Root?.Manager == null) return;
	var dockingManager = _anchorable.Root.Manager;
        //removed original closeCommand
	//dockingManager._ExecuteCloseCommand(_anchorable);
       //replaced by hideCommand
	dockingManager._ExecuteHideCommand(_anchorable);
}

And its worked for me. But i dont know if there are other side effects

Also as @Dirkster99 asked did you noticed that when closing recentFiles previous container is set to LayoutDocumentPanel but when closing FileStats, previous container is set to LayoutDocumentPanel too, but right after is set to LayoutPanel? This seems strange to me and possible source of this exception.

Because when you try to restore layout after, there is wrong LayoutGroup with T type LayoutPanel which cant cast LayoutAnchorable

@Dirkster99
Copy link
Owner

Hi Muhahe,

I've updated the test client to current sources and can still see the Exceptions previously described :-(

Version_03_Edi_RecentFilesTW_AD40_20200304.zip

Replacing the Close with Hide command is a good hint but seems to be a strange solution (because this works for many other workflows).

Maybe there is a better fix such as calling Hide instead of Close under a certain condition?

At this point I am not sure but will try to look into this but am not sure if I can find a better fix either...

@Dirkster99 Dirkster99 added the bug label Mar 5, 2020
@amolf-se
Copy link
Contributor

amolf-se commented Mar 6, 2020

Hi All,

Here is my point of view on this. I think that hiding when close is requested is a bad idea, this breaks the intention. There are already AutoHide, CanClose properties to influence the behavior. If you don't want the anchorable to be closed, set CanClose to false. By using these settings the error is probably bypassed.

The actual error occurs somewhere else. When closing, the CloseInternal function of LayoutContent gets called. It remembers the parent as previouscontainer for reinstation later. on line 706 it checks that if it was the last one in the parent, it moves itself up in the layout tree by setting previouscontainer to it's grandparent (a LayoutPanel), by assumption that the parent will be garbage collected. This will not happen because in the garbage collection routine (LayoutRoot.CollectGarbage) there is an exception for the LayoutDocumentPane of the main view (line 379). The grandparent (LayoutPane) does not accept an LayoutAnchorable or LayoutDocument, because the do not implement ILayoutPanelElement. So in my opinion the code in LayoutContent.CloseInternal should be adapted to keep the parent if it is the 'main' documentpane. I'm not sure this will cover all the situations with floating windows correctly. I hope someone can do the actual changes and various tests.

@Dirkster99
Copy link
Owner

@mkonijnenburg Your analysis is right :-) as usual :-)

I created a branch to make the current solution available for testing and review:
https://github.com/Dirkster99/AvalonDock/tree/Issue_71

I created a new internal bool IsParentRemoved(ILayoutContainer parent) method and its now getting called by the:

  • LayoutRoot.CollectGarbage() code and the
  • 'LayoutContent.CloseInternal()' code

to make this dependency more obvious and ensure consistency in the result.
Otherwise, the move up the parent, grandparent chain is only done if IsParentRemoved indicates that situation 👍 so this fixes the exceptions.

The only thing remaining to close this issue is the broken binding which is visible when I close the Anchorable in the DocumentPane with the (X) button and look at the Menu Entry which shows the Anchorable still being checked :-(

@Dirkster99
Copy link
Owner

Dirkster99 commented Apr 8, 2020

I found a fix in LayoutAnchorable.cs in this commit which is essentally setting the IsVisible property to false when closing the LayoutAnchorable.

When testing, I am finding another related issue which is that the LayoutAnchorable in the DocumentPane reloads without showing the Close (X) button probably because the wrong control is applied in this case(?)

The Close and Close All, Close But this in the Menu Context (which otherwise work) are also not working for the Layout reload case.

Untitled

TBD
I am not sure whether this is a good solution to start with. Users clicking on the (X) of a tool window probably expect a consistent behavior, so we should call HideCommand instead of CloseCommand when clicking the (X) on a LayoutAnchorable in the DocumentPane (See Also Close and Close All But This in context menu), right?

PS'
I tried this feature in all previous versions down to verions 2.0 and it has never workd before. So. its more like implementing a new feature rather than fixing a real but ....

@LyonJack
Copy link
Contributor

LyonJack commented Jul 1, 2020

1593571267(1)
I have try resolve this problem,make sure these code will work

@Dirkster99
Copy link
Owner

I have tested this fix (as it is already commited in master) but it does not solve the problem reference in this workflow. I am not sure how this change could be related to the exceptions that are visible when the above workflow is executed?

@LyonJack
Copy link
Contributor

LyonJack commented Jul 5, 2020

Hi @Dirkster99
This is a mistake of mine. I confused this problem with the one of the others #71

@Dirkster99
Copy link
Owner

Yes this is a confusing issue ticket. The resolution is probably very difficult and involved (which is why I was not able to resolve it yet). From what I saw it looks like:

  • the Visibility binding gets lost when the tool is dragged into the DocumentPane and being closed (which is visible by the chack mark still being visible in the menu entry of the test application - this can be seen then draging the tool in TestApp into the DocumentPane, close it, and try to re-open it with the menu entry)

There also seems to be an inconsistency between Close() and Hide() being applied to the LayoutAnchorable as discussed here. It looks like:

  • Hide is applied when the (X) is clicked with a LayoutAnchorable in its normal side position (in DocumentManager but not in DocumentPane)
  • Close is applied when the (X) is clicked with a LayoutAnchorable in its DocumentPane position in DocumentManager
  • Close is also applied when the (X) is clicked on a floating tool/anchorable window

@Dirkster99
Copy link
Owner

@LyonJack
What Do you mean by this?

This is a mistake of mine. I confused this problem with the one of the others #71

Which one is the other issue (because 71 is the issue you are looking at)?

@LyonJack
Copy link
Contributor

LyonJack commented Jul 5, 2020

Yes, I was confused when dealing with the problem of #71
This problem continues to open

@LyonJack
Copy link
Contributor

image

@LyonJack
Copy link
Contributor

LyonJack commented Jul 11, 2020

Hi @Rasenshurikenbum,
I try to fix this problem in the new version,and I have tried your demo,it works well.Please have a test it's ok now.I dont know the version map with git 3.5.12,so I just fix the new version. @Dirkster99 if this ok you can fix the old version of adock

@LyonJack
Copy link
Contributor

Hi @Dirkster99
I found that you have made the branch management of issue. Next time I solve the corresponding problem, I will switch to the corresponding branch
image

@Dirkster99
Copy link
Owner

I guess the change in #181 does not hurt but it does not fix the problem in this issue :-(

@LyonJack
Please have a look at this workflow - it is important close the document with 'X' since the observed errors will otherwise not occur as mentioned (I've had trouble verifying that too...)

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

No branches or pull requests

5 participants