FilePanel doesn't save/restore state

[humdinger]

This is hrev55602, 64bit.

I'm surprised there isn't a ticket for it yet, because it's been annoying me for years and years.

The file panel always opens with a tiny window. I think it should save its state (size, position, columns) when it closes. There's /boot/home/config/settings/Tracker/FilePanelSettings, but that doesn't appear to be touched at all (or is that just not visible, because it's an attributes-only file?).

[bitigchi]

+1, very annoying bug, along with the read-only window size/position bug.

[pulkomandy]

Indeed the window size and position is supposed to be stored in FilePanelSettings. This is handled by TFilePanel::RestoreWindowState in FilePanelPriv.cpp and BContainerWindow::SaveWindowState in ContainerWindow.cpp (the saving code is common with all Tracker windows).

As far as I can see all the code seems in place for this (both the window size, and the state of the view with columns). So there must be a bug somewhere in it.

[mt]

TFilePanel has its own TFilePanel::SaveState(), So I think we might add TFilePanel::Quit() and call TFilePanel::SaveState() from it.

​https://git.haiku-os.org/haiku/tree/src/kits/tracker/FilePanelPriv.cpp#n916

[dsizzle]

I tried this a little bit - the attrs do get set on the FilePanelSettings file, even if the file remains 0 bytes. I tried it with the Icon-o-Matic open and save dialogs. It seems like it updated the Rect only when I clicked the "Save" or "Open" button, not the "Cancel" button.

See the attached pic (VirtualBox_Haiku_05_02_2022_21_24_01.png); I added some logging and TFilePanel::RestoreWindowState properly gets the rect attr, and the open dialog seems to be about the right size.

[Jim906]

I can work on this.

Just to make sure, is the goal is to have the FilePanel use the same settings (position, size, columns) universally, regardless of which application displays it? Or do you want to save and restore separate settings for each application that uses a FilePanel?

[humdinger]

Or do you want to save and restore separate settings for each application that uses a FilePanel?

First I'd have said "Keep it simple, sunshine", but thinking about it, having the attribute columns saved per app basis would be neat. Maybe, to avoid cruft accumulating over time, settings for no longer installed apps should be removed. I suppose it'll all be tracked by app signature?

Should last used path be saved as well? I'd say yes, if it's not overriden by the application creating the file panel.

[pulkomandy]

I'd say let's start simple and debug the existing code that is already supposed to do this (with a single setting for all apps). We can make it more complicated later.

[Jim906]

Patch under review: ​https://review.haiku-os.org/c/haiku/+/5433.

This is a simple bug fix as pulkomandy recommended. Maybe in the future I can try to add separate settings for each application.

I think this fixes the main problem with BFilePanel itself, but this is not a perfect fix because of the different ways that applications manage their BFilePanel objects.

Changes made to the 'Save' object might not be immediately (before closing the application) applied to 'Open' and vice versa. Icon-O-Matic will immediately update Save to match changes to Open. When Icon-O-Matic closes, it saves the Open settings first, and then overwrites these with the Save settings. Because Save was already synced with Open, nothing is lost.

In StyledEdit, the Open settings are saved last, and there is no mechanism to sync the 2 objects, so the Save settings are lost. In Pe, it is the opposite: the Save settings overwrite the Open settings.

mt's comment mentioned the TFilePanel::SaveState function. It looks like this is being called already by BContainerWindow::Quit, so it wasn't necessary to add TFilePanel::Quit.

[waddlesplash]

Perhaps we should have separate settings for Open and Save file panels, broadly speaking?

[pulkomandy]

One simple thing to do is save the settings to the file only if they were modified during the panel lifetime (the panel was resized, or some columns were changed).

This will avoid overwriting the resized panel settings with a panel that was possibly never shown and used, and still has default settings.

I think this is a simple enough change and it will keep the behavior understandable. Having multiple settings for different panels is a bit less easy to understand for users, sometimes things will look as if they have a mind of their own and do non-obvious things.

[Jim906]

Replying to pulkomandy:

One simple thing to do is save the settings to the file only if they were modified during the panel lifetime (the panel was resized, or some columns were changed).

There is an existing function, BContainerWindow::StateNeedsSaving, that I might be able to modify to accomplish this. Currently it seems to err on the side of returning True, which leads to saving state even at times when the user has not made any changes to the window's settings.

[Jim906]

I uploaded another patch that is intended to prevent saving if the panel was not modified (by the user): ​https://review.haiku-os.org/c/haiku/+/5447.

I say 'by the user' because one factor seems to be that a member object can be constructed, and then modified by the parent constructor (or by another parent function like Init), which triggers the need to save that member (and, indirectly, the need to save the parent) even if the user never touched the panel.

[diver]

Just noticed that /bin/filepanel window doesn't save its size.

[Jim906]

Replying to diver:

Just noticed that /bin/filepanel window doesn't save its size.

It seems to save its size on my system, running 64-bit hrev56386.

As it stands now, the save occurs when the panel is closed, not immediately upon a size change. If you run bin/filepanel, change the window size, and then run another instance of bin/filepanel before closing the first one, then your change to the first window would not be replicated in the second window. Does that maybe explain what you're seeing?

[diver]

I would expect it to remember its size after closing and opening new filepanel.

[diver]

Odd, now it works as expected.

[waddlesplash]

All mentioned patches were merged. Any remaining items here? I think this ticket can be considered fixed and closed?

[humdinger]

I think so, too. I'll open a new enhancement ticket for the idea to store size/position and attribute column layout per app_signature.