Question regarding Session management and toolbar state (ticket 7099)

[Stephan Witt]

But somehow there is a connection between visibility flags and the
toolbar type, isn't it? The allowauto_ flag depends on visibility.
This looks a little bit weird.

Yes, this is just an example of bad programming practice.
In the Visibility enum, we are now mixing up flags indicating where to
show and when to show a toolbar. This seems bad, but ok.
But then GuiToolbar::setVisibility says that all items in the
Visibility enum >= Toolbars::MATH are auto_toggling. This is just bad,
because someone else adds a new item to the Visibility enum and can't
possibly know that somewhere else in the code, this new item implies a
different behaviour because it's added to the end.
So, when the SAMEROW flag is set, we also set allow_auto_, which is a BUG.
{{{
...
enum Visibility {
....
ALLOWAUTO = MATH | TABLE | REVIEW | MATHMACROTEMPLATE;
...
}
allowauto_ = visibility & Toolbars::ALLOWAUTO;
}}}

Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX.

The wrong settings are saved again?
Perhaps it's not a failure of restoreSession but a save of wrong
toolbar settings. I cannot tell for sure.

You can find out if you can reproduce the bug ;).

Hmm...
I'm sure in the saved preferences all toolbar visibility settings
had a 0 assigned when I had the problem. And I can trigger the
misbehavior when saving a 0 with the property list editor.

In any case it shouldn't happen that visibility is restored as 0.

No, if we can't restore from QSettings, we should set the visibility
to the default I guess.

I've prepared a patch for this based on your proposal.
The removed private name_ member in GuiToolbar.h isn't used anywhere.

@Abdel: I don't have the energy to start with a git repository somewhere.
Either I apply it to svn or you or Vincent take over.

The patch is attached.

Stephan

[Abdelrazak Younes]

I think the simplest solution would be to always call restore the toolbars
following the toolbars.ui settings and disregard the session management.
This would have the side effect that any moved toolbars state will be lost
as the toolbars will be restored in their default state.

Why would we do that ?
The bug is that _sometimes_ we lose the toolbar settings, and you
propose to fix it such that we _always_ lose it :S..

I said "simplest solution" :-)
I didn't say this is the _right_ solution.

Abdel.

[Vincent van Ravesteijn]

Post by Abdelrazak Younes
I said "simplest solution" :-)
I didn't say this is the _right_ solution.

Ok, I'll try to remember this lesson.

I often fail to see the simplest solution to problems. Now I know why.

Vincent

[Abdelrazak Younes]

First, there are two different types of settings of toolbars: the
location of the toolbars, and the visibility of the toolbars. The
former is managed by Qt in QMainWindow::saveState()/restoreState().
The latter is done by LyX in
GuiToolbar::saveSession()/restoreSession(). The reason for this
distinction is that Qt has no knowledge about the three-way visibility
setting we have in LyX.

What I forgot to say is: Why don't we implement saveState() and
restoreState() in GuiToolbar which overrides QToolbar::saveState() and
restoreState(). Then restoring the visibility of the toolbars is done
by QMainWindow::restoreState. (At least, that is what I would think).

Yes, that's probably the way to go. Stefan, Vincent, please use the
toolbar-settings git branch if you work on that.

Abdel.

[Vincent van Ravesteijn]

Post by Abdelrazak Younes

What I forgot to say is: Why don't we implement saveState() and
restoreState() in GuiToolbar which overrides QToolbar::saveState() and
restoreState(). Then restoring the visibility of the toolbars is done
by QMainWindow::restoreState. (At least, that is what I would think).

Yes, that's probably the way to go. Stefan, Vincent, please use the
toolbar-settings git branch if you work on that.

If you uncheck "restore window layouts and geometries" in Preferences,
you still want to restore the toolbar's visibility settings, though we
don't call QMainWindow::restoreState(). That might be a reason to not
do it.

Vincent

[Abdelrazak Younes]

Post by Vincent van Ravesteijn

Post by Abdelrazak Younes

What I forgot to say is: Why don't we implement saveState() and
restoreState() in GuiToolbar which overrides QToolbar::saveState() and
restoreState(). Then restoring the visibility of the toolbars is done
by QMainWindow::restoreState. (At least, that is what I would think).

Yes, that's probably the way to go. Stefan, Vincent, please use the
toolbar-settings git branch if you work on that.

If you uncheck "restore window layouts and geometries" in Preferences,
you still want to restore the toolbar's visibility settings, though we
don't call QMainWindow::restoreState(). That might be a reason to not
do it.

Well, in that case, the settings from the ui file will be used IIRC.

Abdel.

[Stephan Witt]

Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX. Why are the settings
not stored properly when exiting LyX ?

AFAIR LyX checks the time tag of "ui/default.ui" and use the toolbar information from there instead of QSettings if the file was changed.
Perhaps this file has a time tag in the future in the Windows installer?

Ha! That's it.

The combination of changed ui file and already stored settings is the culprit. Thanks!

In GuiApplication::readUIFile() the settings for views gets removed if the
ui file is newer than the saved timestamp in the settings.
Later the non existing settings will be used to initialize the visibility.
The default value for a non existing setting is 0.

So, Vincents (pending) patch should catch that already.

Stephan

[Vincent van Ravesteijn]

Post by Stephan Witt
Ha! That's it.
The combination of changed ui file and already stored settings is the culprit. Thanks!
In GuiApplication::readUIFile() the settings for views gets removed if the
ui file is newer than the saved timestamp in the settings.
Later the non existing settings will be used to initialize the visibility.
The default value for a non existing setting is 0.
So, Vincents (pending) patch should catch that already.

So, the settings that we read either do not exist, or won't be 0.

So, if I apply the (pending) patch, the bug can be closed as fixed ?

Vincent

[Vincent van Ravesteijn]

I see. Another possibility: toggle fullscreen.
Stephan

No I don't think so.

GuIView::resetDialogs() call "constructToolbars();". So, only in this
case it is absolutely necessary that we have saved the toolbar
settings, otherwise they are lost during the construction of the
toolbars.

Vincent

[Stephan Witt]

Post by Vincent van Ravesteijn

Post by Stephan Witt
Ha! That's it.
The combination of changed ui file and already stored settings is the culprit. Thanks!
In GuiApplication::readUIFile() the settings for views gets removed if the
ui file is newer than the saved timestamp in the settings.
Later the non existing settings will be used to initialize the visibility.
The default value for a non existing setting is 0.
So, Vincents (pending) patch should catch that already.

So, the settings that we read either do not exist, or won't be 0.

Yes, if I touch the default ui file the visibility gets the error_val.

Post by Vincent van Ravesteijn
So, if I apply the (pending) patch, the bug can be closed as fixed ?

I say yes.

Stephan

[Vincent van Ravesteijn]

Post by Stephan Witt

Post by Stephan Witt
In GuiApplication::readUIFile() the settings for views gets removed if the
ui file is newer than the saved timestamp in the settings.
Later the non existing settings will be used to initialize the visibility.
The default value for a non existing setting is 0.

I say yes.

But we are not there yet.

My fix shouldn't be necessary as GuiView::restoreLayout() will return
false when the settings for the views are removed due to the
following:

{{{
bool GuiView::restoreLayout() {
[...]
if (!settings.contains(icon_key))
return false;
[...]
}
}}}

If it returns false, initToolbars() will be called and the values get
the original value.

BUT, I have found the following recipe by looking at the sourcecode

1. Modify default.ui,
2. Run LyX (without the patch of course),
3. Modify Preferences,
- Now resetDialogs() calls saveLayout() and restoreLayout(), but
saveLayout doesn't save the toolbar and window setting. Then
restoreLayout restores everything to 0.
4. Close LyX.
- Now, the data in the registry is all 0.

Vincent

[Stephan Witt]

Post by Vincent van Ravesteijn

Post by Stephan Witt

Post by Stephan Witt
In GuiApplication::readUIFile() the settings for views gets removed if the
ui file is newer than the saved timestamp in the settings.
Later the non existing settings will be used to initialize the visibility.
The default value for a non existing setting is 0.

I say yes.

But we are not there yet.
My fix shouldn't be necessary as GuiView::restoreLayout() will return
false when the settings for the views are removed due to the
{{{
bool GuiView::restoreLayout() {
[...]
if (!settings.contains(icon_key))
return false;
[...]
}
}}}

Yes, I see it now.

Post by Vincent van Ravesteijn
If it returns false, initToolbars() will be called and the values get
the original value.
BUT, I have found the following recipe by looking at the sourcecode
1. Modify default.ui,
2. Run LyX (without the patch of course),
3. Modify Preferences,
- Now resetDialogs() calls saveLayout() and restoreLayout(), but
saveLayout doesn't save the toolbar and window setting. Then
restoreLayout restores everything to 0.
4. Close LyX.
- Now, the data in the registry is all 0.

I see. Another possibility: toggle fullscreen.

Stephan

[Vincent van Ravesteijn]

Post by Stephan Witt

But somehow there is a connection between visibility flags and the
toolbar type, isn't it? The allowauto_ flag depends on visibility.
This looks a little bit weird.

Yes, this is just an example of bad programming practice.
In the Visibility enum, we are now mixing up flags indicating where to
show and when to show a toolbar. This seems bad, but ok.
But then GuiToolbar::setVisibility says that all items in the
Visibility enum >= Toolbars::MATH are auto_toggling. This is just bad,
because someone else adds a new item to the Visibility enum and can't
possibly know that somewhere else in the code, this new item implies a
different behaviour because it's added to the end.
So, when the SAMEROW flag is set, we also set allow_auto_, which is a BUG.
{{{
...
enum Visibility {
....
ALLOWAUTO = MATH | TABLE | REVIEW | MATHMACROTEMPLATE;
...
}
allowauto_ = visibility & Toolbars::ALLOWAUTO;
}}}

Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX.

The wrong settings are saved again?
Perhaps it's not a failure of restoreSession but a save of wrong
toolbar settings. I cannot tell for sure.

You can find out if you can reproduce the bug ;).

Hmm...
I'm sure in the saved preferences all toolbar visibility settings
had a 0 assigned when I had the problem. And I can trigger the
misbehavior when saving a 0 with the property list editor.

In any case it shouldn't happen that visibility is restored as 0.

No, if we can't restore from QSettings, we should set the visibility
to the default I guess.

I've prepared a patch for this based on your proposal.
The removed private name_ member in GuiToolbar.h isn't used anywhere.

I already removed the allowauto_ member completely.

Post by Stephan Witt
- if (visibility >= MATH) {
+ if (visibility & ALLOWAUTO) {

Thanks, I forgot this one apparently.

Post by Stephan Witt
Index: src/frontends/qt4/GuiToolbar.cpp
===================================================================
--- src/frontends/qt4/GuiToolbar.cpp (Revision 37067)
+++ src/frontends/qt4/GuiToolbar.cpp (Arbeitskopie)
@@ -90,8 +90,9 @@
void GuiToolbar::setVisibility(int visibility)
{
- visibility_ = visibility;
- allowauto_ = visibility_ >= Toolbars::MATH;
+ guiApp->toolbars().defaultVisibility(fromqstr(objectName()));
+ allowauto_ = visibility_ & Toolbars::ALLOWAUTO;
}

I'd prefer to do this in GuiToolbar::restoreSession().


void GuiToolbar::restoreSession()
{
QSettings settings;

setVisibility(settings.value(sessionKey() + "/visibility").toInt());
}

Post by Stephan Witt
@Abdel: I don't have the energy to start with a git repository somewhere.
Either I apply it to svn or you or Vincent take over.

It doesn't matter. For now it does not interfere with Abdel's patch.

But I'll commit the things in this patch.

But this doesn't fix the bug yet right ?

Vincent

[Vincent van Ravesteijn]

Post by Vincent van Ravesteijn
void GuiToolbar::restoreSession()
{
       QSettings settings;
       setVisibility(settings.value(sessionKey() + "/visibility").toInt());
}

void GuiToolbar::restoreSession()
{
QSettings settings;
int const error_val = -1;
int visibility = settings.value(sessionKey() + "/visibility",
error_val).toInt();
if (visibility == error_val) {
LYXERR0("Settings are screwed up! Falling back to the defaults.");
visibility =
guiApp->toolbars().defaultVisibility(fromqstr(objectName()));
}
setVisibility(visibility);
}

This is what I wanted to propose.

Vincent

[Stephan Witt]

Post by Vincent van Ravesteijn

Post by Vincent van Ravesteijn
void GuiToolbar::restoreSession()
{
QSettings settings;
setVisibility(settings.value(sessionKey() + "/visibility").toInt());
}

void GuiToolbar::restoreSession()
{
QSettings settings;
int const error_val = -1;
int visibility = settings.value(sessionKey() + "/visibility",
error_val).toInt();
if (visibility == error_val) {
LYXERR0("Settings are screwed up! Falling back to the defaults.");
visibility =
guiApp->toolbars().defaultVisibility(fromqstr(objectName()));
}
setVisibility(visibility);
}
This is what I wanted to propose.

In principle this would be fine.

I would add the name of the toolbar to the error message.
And if the settings were written with the default value for visibility_ (0)
then restoreSession() happily will restore that nonsense value.

Stephan

PS: I forgot to mention again the change below...

Index: src/frontends/qt4/GuiToolbar.h
===================================================================
--- src/frontends/qt4/GuiToolbar.h (Revision 37069)
+++ src/frontends/qt4/GuiToolbar.h (Arbeitskopie)
@@ -112,8 +112,6 @@
void showEvent(QShowEvent *);

///
- QString name_;
- ///
QList<Action *> actions_;
/// initial visibility flags
int visibility_;

[Vincent van Ravesteijn]

Post by Stephan Witt
I would add the name of the toolbar to the error message.

Ok.

Post by Stephan Witt
And if the settings were written with the default value for visibility_ (0)
then restoreSession() happily will restore that nonsense value.

You are right. But we should never write a value of 0.

Do you already have an idea how that can happen ?

Post by Stephan Witt
PS: I forgot to mention again the change below...

That's why I already committed that in r37070.

Vincent

[Abdelrazak Younes]

Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX. Why are the settings
not stored properly when exiting LyX ?

AFAIR LyX checks the time tag of "ui/default.ui" and use the toolbar
information from there instead of QSettings if the file was changed. Perhaps
this file has a time tag in the future in the Windows installer?

You might want to have a look at it.
/// initial visibility flags
int visibility_;
Why is it only the initial flags ?

Because the "default.ui" file is only read once if there's no session
saved. Afterwards the visibility is handled by Qt session management.
This is the root of the problem as Qt doesn't know about the auto
visibility, a toolbar is on or off. That's why your idea of
reimplementing saveState() and restoreState() is good.

Anyway, the visibility_ variable is not changed in
void GuiToolbar::saveSession() const
{
QSettings settings;
settings.setValue(sessionKey() + "/visibility", visibility_);
}
we ALWAYS write the original values that were read. Whenever we can't
find the setting in restoreSession, we read a 0, and we will thus save
a 0 again and read a 0 again even if the user has changed the setting
in the meantime.
This is the cause of the bug I guess.

I see.

Now, let me tell you what I would do if I had the time (and what I
started to do actually in the git branch):

1) get rid completely of the "Toolbars" section in default.ui
visibility and positioning are fundamentally session stuff.

2) deliver a lyx.ini file for default visibility and positioning, not
only for toolbars.

3) handle the the visibility and positioning completely in
GuiToolbar::saveState() and restoreState() using QSettings: try the
system QSettings first andf if empty use lyx.ini

4) Let the new ToolbarSettings dialog adjust these QSettings.

Abdel.

[Vincent van Ravesteijn]

But somehow there is a connection between visibility flags and the
toolbar type, isn't it? The allowauto_ flag depends on visibility.
This looks a little bit weird.

Yes, this is just an example of bad programming practice.

In the Visibility enum, we are now mixing up flags indicating where to
show and when to show a toolbar. This seems bad, but ok.

But then GuiToolbar::setVisibility says that all items in the
Visibility enum >= Toolbars::MATH are auto_toggling. This is just bad,
because someone else adds a new item to the Visibility enum and can't
possibly know that somewhere else in the code, this new item implies a
different behaviour because it's added to the end.

So, when the SAMEROW flag is set, we also set allow_auto_, which is a BUG.

I guess allowauto_ should be something like:

{{{
...
enum Visibility {
....
ALLOWAUTO = MATH | TABLE | REVIEW | MATHMACROTEMPLATE;
...
}

allowauto_ = visibility & Toolbars::ALLOWAUTO;

}}}

Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX.

The wrong settings are saved again?
Perhaps it's not a failure of restoreSession but a save of wrong
toolbar settings. I cannot tell for sure.

You can find out if you can reproduce the bug ;).

In any case it shouldn't happen that visibility is restored as 0.

No, if we can't restore from QSettings, we should set the visibility
to the default I guess.

Vincent

[Vincent van Ravesteijn]

I encountered the problem yesterday too after recompile and restart without updating the resources.
So I tried to understand the cause and I don't understand the code in GuiView::restoreLayout().
As I read the code the call of initToolbars() depends on the success of the restoreState() call
for the GuiView widget when lyxrc.allow_geometry_session is true.
The toolbars are constructed already at this point and used the QSettings session state on their own
in GuiView::constructToolbars(), GuiToolbar::GuiToolbar() and GuiToolbar::restoreSession().
In case of an inconsistency the initialization of toolbar visibility via session management
fails silently but the restoreState() call mentioned above succeeds. Now the visibility of all
toolbars is 0.

I'm not sure what it is you have to know, but let me explain some things.

First, there are two different types of settings of toolbars: the
location of the toolbars, and the visibility of the toolbars. The
former is managed by Qt in QMainWindow::saveState()/restoreState().
The latter is done by LyX in
GuiToolbar::saveSession()/restoreSession(). The reason for this
distinction is that Qt has no knowledge about the three-way visibility
setting we have in LyX.

This means, if it fails to read the toolbar visibility in
GuiToolbar::restoreSession() (using QSettings) there is absolutely no
way to restore it. I think this is in no way dependent on
QMainWindow::restoreState() and GuiView::initToolbars().

Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX. Why are the settings
not stored properly when exiting LyX ? Why the hassle of hacking the
registry. Besides, I thought this was a windows installer issue. Are
you sure you saw exactly the same bug ?


Vincent

[Vincent van Ravesteijn]

First, there are two different types of settings of toolbars: the
location of the toolbars, and the visibility of the toolbars. The
former is managed by Qt in QMainWindow::saveState()/restoreState().
The latter is done by LyX in
GuiToolbar::saveSession()/restoreSession(). The reason for this
distinction is that Qt has no knowledge about the three-way visibility
setting we have in LyX.

What I forgot to say is: Why don't we implement saveState() and
restoreState() in GuiToolbar which overrides QToolbar::saveState() and
restoreState(). Then restoring the visibility of the toolbars is done
by QMainWindow::restoreState. (At least, that is what I would think).

Vincent

[Abdelrazak Younes]

Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX. Why are the settings
not stored properly when exiting LyX ?

AFAIR LyX checks the time tag of "ui/default.ui" and use the toolbar
information from there instead of QSettings if the file was changed.
Perhaps this file has a time tag in the future in the Windows installer?

Abdel.

[Vincent van Ravesteijn]

Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX. Why are the settings
not stored properly when exiting LyX ?

AFAIR LyX checks the time tag of "ui/default.ui" and use the toolbar
information from there instead of QSettings if the file was changed. Perhaps
this file has a time tag in the future in the Windows installer?

You might want to have a look at it.

In GuiToolbar.h I see:

/// initial visibility flags
int visibility_;

Why is it only the initial flags ?

Anyway, the visibility_ variable is not changed in
GuiToolbar::toggle(), so when the session gets saved in:

void GuiToolbar::saveSession() const
{
QSettings settings;
settings.setValue(sessionKey() + "/visibility", visibility_);
}

we ALWAYS write the original values that were read. Whenever we can't
find the setting in restoreSession, we read a 0, and we will thus save
a 0 again and read a 0 again even if the user has changed the setting
in the meantime.

This is the cause of the bug I guess.

Vincent

[Stephan Witt]

Post by Vincent van Ravesteijn

I encountered the problem yesterday too after recompile and restart without updating the resources.
So I tried to understand the cause and I don't understand the code in GuiView::restoreLayout().
As I read the code the call of initToolbars() depends on the success of the restoreState() call
for the GuiView widget when lyxrc.allow_geometry_session is true.
The toolbars are constructed already at this point and used the QSettings session state on their own
in GuiView::constructToolbars(), GuiToolbar::GuiToolbar() and GuiToolbar::restoreSession().
In case of an inconsistency the initialization of toolbar visibility via session management
fails silently but the restoreState() call mentioned above succeeds. Now the visibility of all
toolbars is 0.

I'm not sure what it is you have to know, but let me explain some things.
First, there are two different types of settings of toolbars: the
location of the toolbars, and the visibility of the toolbars. The
former is managed by Qt in QMainWindow::saveState()/restoreState().
The latter is done by LyX in
GuiToolbar::saveSession()/restoreSession(). The reason for this
distinction is that Qt has no knowledge about the three-way visibility
setting we have in LyX.
This means, if it fails to read the toolbar visibility in
GuiToolbar::restoreSession() (using QSettings) there is absolutely no
way to restore it. I think this is in no way dependent on
QMainWindow::restoreState() and GuiView::initToolbars().

Thanks for the explanation.
I asked for the big picture... so thank you.

But somehow there is a connection between visibility flags and the
toolbar type, isn't it? The allowauto_ flag depends on visibility.
This looks a little bit weird.

Post by Vincent van Ravesteijn
Furthermore, I'm not sure yet what causes the bug, because it seems
that if the user sets the toolbar visibility from within LyX, this
setting is lost again on the new start-up of LyX.

The wrong settings are saved again?

Perhaps it's not a failure of restoreSession but a save of wrong
toolbar settings. I cannot tell for sure.

Post by Vincent van Ravesteijn
Why are the settings not stored properly when exiting LyX ?
Why the hassle of hacking the registry.

It's not hacking the registry AFAICS. The Qt-Settings are saved
that way - on Windows. On a Mac it's a property list file in
$HOME/Library/Preferences. The name is derived from domain and
application name.

Post by Vincent van Ravesteijn
Besides, I thought this was a windows installer issue. Are
you sure you saw exactly the same bug ?

No, I'm not sure. But the fix could be helpful for both?

After doing a clean build with updated resources the problem
vanished. But something is fishy here...

In any case it shouldn't happen that visibility is restored as 0.

Stephan

[Abdelrazak Younes]

See http://www.lyx.org/trac/ticket/7099
I encountered the problem yesterday too after recompile and restart without updating the resources.
So I tried to understand the cause and I don't understand the code in GuiView::restoreLayout().
As I read the code the call of initToolbars() depends on the success of the restoreState() call
for the GuiView widget when lyxrc.allow_geometry_session is true.
The toolbars are constructed already at this point and used the QSettings session state on their own
in GuiView::constructToolbars(), GuiToolbar::GuiToolbar() and GuiToolbar::restoreSession().
In case of an inconsistency the initialization of toolbar visibility via session management
fails silently but the restoreState() call mentioned above succeeds. Now the visibility of all
toolbars is 0.
I want to improve the situation somehow. Otherwise the user has to go out to find the
store for session data and remove it manually. This should fix the problem in ticket 7099.
Does anyone has some knowledge about session management code?

I think the simplest solution would be to always call restore the
toolbars following the toolbars.ui settings and disregard the session
management. This would have the side effect that any moved toolbars
state will be lost as the toolbars will be restored in their default state.

Please see also my git branch in gitorious.

Abdel.

[Vincent van Ravesteijn]

I think the simplest solution would be to always call restore the toolbars
following the toolbars.ui settings and disregard the session management.
This would have the side effect that any moved toolbars state will be lost
as the toolbars will be restored in their default state.

Why would we do that ?

The bug is that _sometimes_ we lose the toolbar settings, and you
propose to fix it such that we _always_ lose it :S..

Vincent