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

[Bug]: Docking Persistence broken since build ##.23.10.303 #1381

Closed
stigzler opened this issue Mar 30, 2024 · 16 comments
Closed

[Bug]: Docking Persistence broken since build ##.23.10.303 #1381

stigzler opened this issue Mar 30, 2024 · 16 comments
Labels
area:docking All issues to do with the docking. bug Something isn't working completed This issue has been completed. regression Something was working in a previous release, but isn't working now. version:85-lts All things to do with V85 LTS.

Comments

@stigzler
Copy link

stigzler commented Mar 30, 2024

Describe the bug
This was working fine in a 7.xx library. However, since updating, persistence now broken.
If one saves the docking states via:

dockingManager.SaveConfigToArray()

and then tries to reload it via:

dockingManager.LoadConfigFromArray([//the byte array])

then lots of windows are missing/hidden.

Example of full method used to save/retrieve window states:

Properties.Settings.Default.DockingPersistenceString = Convert.ToBase64String(dockingManager.SaveConfigToArray());
Properties.Settings.Default.Save();
// stuff
dockingManager.LoadConfigFromArray(Convert.FromBase64String(Properties.Settings.Default.DockingPersistenceString));

To Reproduce
I have made a simple Solution demonstrating the problem. Find it here:
Demo Git Page

  1. Open app
  2. Move the middle windows about
  3. Click "Save Windows" (disc icon)
  4. Click "Show docking persistence string" (eye) to make sure this has been saved into settings
  5. Move the windows about again.
  6. Click "Load Windows" (folder icon)

Expected behaviour
The windows re-arrange themselves to the saved state.

Actual behaviour
Windows missing and in wrong locations.

Desktop (please complete the following information):

  • OS: [e.g. Windows 11]
  • Version: ?
  • Framework/.NET Version: 4.7.2
  • Toolkit Version: 80.24.3.64
@stigzler stigzler added the bug Something isn't working label Mar 30, 2024
@Smurf-IV
Copy link
Member

Duplicate of #1310 - but this has a demo item ;-) - so keeping this one !

@Smurf-IV Smurf-IV added the regression Something was working in a previous release, but isn't working now. label Mar 31, 2024
@Smurf-IV
Copy link
Member

@Wagnerp Is this a candidate for 8.5 ?

@PWagner1
Copy link
Contributor

PWagner1 commented Apr 1, 2024

@Wagnerp Is this a candidate for 8.5 ?

@Smurf-IV Yes it is

@Smurf-IV Smurf-IV added this to the Version 85 milestone Apr 1, 2024
@Smurf-IV Smurf-IV self-assigned this Apr 5, 2024
@Smurf-IV Smurf-IV added the under investigation This bug/issue is currently under investigation. label Apr 5, 2024
@Smurf-IV
Copy link
Member

Smurf-IV commented Apr 5, 2024

Using the Docking Persistence 2022 demo application..
Works in :

  • 80.23.10.296-beta

Broken in :

  • 90.23.11.326-beta
  • 80.23.11.318- beta. this has a sizing problem! (@Wagnerp and should be removed from the nuget distro!)
  • 80.23.11.321- beta

@Smurf-IV
Copy link
Member

Smurf-IV commented Apr 5, 2024

Works in :

  • 80.23.10.296-alpha

Broken in:

  • 90.23.10.303-alpha

@Smurf-IV
Copy link
Member

Smurf-IV commented Apr 5, 2024

@Wagnerp There seems to be a lot of Commits missing in the alpha branch for the indicated times:
image

@Smurf-IV Smurf-IV changed the title [Bug]: Docking Persistence broken in new version [Bug]: Docking Persistence broken since build ##.23.10.303 Apr 6, 2024
@Smurf-IV Smurf-IV added completed This issue has been completed. and removed under investigation This bug/issue is currently under investigation. labels Apr 6, 2024
@Smurf-IV Smurf-IV closed this as completed Apr 6, 2024
@Smurf-IV Smurf-IV removed their assignment Apr 6, 2024
@lukas2-werner
Copy link

For me the docking persistence is still broken with the latest canary on commit a6788ea
After stepping through the code it seems that this line is what prevents any docked page to be loaded from a configuration:


This null check was introduced in commit 56c44e8 and as far as I can see, is not necessary because LoadChildDockingElement already properly deals with nulls
protected virtual void LoadChildDockingElement(XmlReader xmlReader,
KryptonPageCollection pages,
IDockingElement? child)
{
if (child != null)
{
child.LoadElementFromXml(xmlReader, pages);
}
else
{
var nodeName = xmlReader.Name;
do
{
// Read past this element
if (!xmlReader.Read())
{
throw new ArgumentException(@"An element was expected but could not be read in.", nameof(xmlReader));
}
// Finished when we hit the end element matching the incoming one
if ((xmlReader.NodeType == XmlNodeType.EndElement) && (xmlReader.Name == nodeName))
{
break;
}
} while (true);
}
}

If I remove the null check my configuration is correctly loaded

@PWagner1 PWagner1 reopened this Apr 24, 2024
@PWagner1 PWagner1 added area:docking All issues to do with the docking. and removed completed This issue has been completed. labels Apr 24, 2024
@PWagner1
Copy link
Contributor

@Smurf-IV Is it possible to set child as [DisallowNull]?

@lukas2-werner
Copy link

I also just noticed, that the fix 593a5f5 referenced above in this issue introduced a new bug.

Several call-sites call PropogateAction with null for the uniqueNames parameter. This results in countToUse evaluate to 0 and the for loop is never executed effectively ignoring the action.
E.g. if I drag a single docked page my application crashes in this line

throw new ArgumentOutOfRangeException(nameof(pages), @"Cannot perform operation with a page that is already present inside docking hierarchy");

Krypton.Docking.dll!Krypton.Docking.DockingElement.DemandPagesNotBePresent(Krypton.Navigator.KryptonPage[] pages) Line 574
Krypton.Docking.dll!Krypton.Docking.KryptonDockingSpace.Append(Krypton.Navigator.KryptonPage[] pages) Line 61
Krypton.Docking.dll!Krypton.Docking.KryptonDockingManager.DoDragDrop(System.Drawing.Point screenPoint, System.Drawing.Point elementOffset, System.Windows.Forms.Control c, Krypton.Navigator.KryptonPageCollection pages) Line 3167
Krypton.Docking.dll!Krypton.Docking.KryptonDockingDockspace.OnDockspaceBeforePageDrag(object sender, Krypton.Navigator.PageDragCancelEventArgs e) Line 505
Krypton.Workspace.dll!Krypton.Workspace.KryptonWorkspace.OnBeforePageDrag(Krypton.Navigator.PageDragCancelEventArgs de)
Krypton.Workspace.dll!Krypton.Workspace.KryptonWorkspace.InternalPageDragStart(object sender, Krypton.Navigator.KryptonNavigator navigator, Krypton.Navigator.PageDragCancelEventArgs e)

If I had to guess its the DockingMultiUpdate in KryptonDockingManager.DoDragDrop that is being ignored.

@Smurf-IV
Copy link
Member

@lukas2-werner Thanks for testing all this docking stuff..
Do you have an example (Or is it reproducible with an existing demo) of these issues please ?

@Smurf-IV Smurf-IV added the awaiting feedback A fix for this issue has been implemented, waiting for feedback on the fix. label Apr 28, 2024
@lukas2-werner
Copy link

Both can be reproduced with the Docking Persistence example.

This crash happens as soon as I drag one of the docked pages in the example.

The docking persistence bug I can reproduce with the following steps:

  1. Start Docking Persistence Example
  2. Save layout as file
  3. Close all docked pages
  4. Load layout file
    -> Docked pages are not loaded

If I remove the null check


the docked pages are loaded correctly

@Smurf-IV Smurf-IV removed the awaiting feedback A fix for this issue has been implemented, waiting for feedback on the fix. label Apr 30, 2024
@Smurf-IV
Copy link
Member

@lukas2-werner Thanks for the Steps..
When I was doing the testing I did not do Step 3, I was just moving the pages around, and having 2 save points instead.

@SteveAndrews
Copy link

When I comment if (child != null), I then get an argument exception in KryptonDockingSpace.SpaceControl that Should only ever set the value once. I'm not modifying the saved file.

private void OnKryptonTestForm_Load(object? sender, EventArgs e)
{
    var appDataPath = Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData);
    var fileNameAndPath = Path.Combine(appDataPath, "TestKryptonApp", "layout.xml");

    if (File.Exists(fileNameAndPath))
    {
        this.kryptonDockingManager1.LoadConfigFromFile(fileNameAndPath);
    }
}

private void OnKryptonTestForm_FormClosing(object? sender, FormClosingEventArgs e)
{
    var appDataPath = Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData);
    var fileNameAndPath = Path.Combine(appDataPath, "TestKryptonApp", "layout.xml");

    this.kryptonDockingManager1.SaveConfigToFile(fileNameAndPath);
}

@Smurf-IV
Copy link
Member

Darn it.. I have been chasing my tail on this..
I was expecting the DockingPersitence sample to restore the navigator items in the middle if they have been closed, I even went back through the code to V6, and then ran sample installer for V6:
image
Turns out it does NOT do that!!

It restores everything but the closed middle items !

So I'll stop that search, and get back to not throwing exceptions!

@stigzler
Copy link
Author

Just to say thanks for all your work on this - it's looking like a pest! Let me know anything I can do

@Smurf-IV
Copy link
Member

Just to say thanks for all your work on this - it's looking like a pest! Let me know anything I can do

Fix is in for alpha (Please wait for the merge)
I'll do 8.5 iff this proves to be the fix ;-)

@PWagner1 PWagner1 added the version:85-lts All things to do with V85 LTS. label Jun 21, 2024
@Smurf-IV Smurf-IV removed their assignment Jun 22, 2024
@Smurf-IV Smurf-IV added completed This issue has been completed. and removed under investigation This bug/issue is currently under investigation. labels Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:docking All issues to do with the docking. bug Something isn't working completed This issue has been completed. regression Something was working in a previous release, but isn't working now. version:85-lts All things to do with V85 LTS.
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

5 participants