Version: 2.2 (using 4.1.68 (KDE 4.1.68 (KDE 4.2 >= 20080926)), compiled sources) Compiler: gcc OS: Linux (i686) release 2.6.25-2-686 First note that I can reproduce this bug in both 4.1 branch and trunk (4.2) To reproduce; start a konsole start a second konsole window. Scale window one to be a little smaller. Create a new tab in the second konsole. What happens is that I see the second window with its new tab change size. It takes on the size of the last resized konsole window. What I expected is that konsole never resizes my window automatically after the initial opening of the window.
Ah, -that- is why my konsole window keeps resizing itself. Good catch. The question is where is the applyMainWindowSettings call that triggers the resizing (can't be the one called by setAutoSaveSettings, since that's done only when creating a new window).
I can confirm this. I wish Bugzilla wouldn't automatically confirm bugs from certain users.
Ahhh, so once again I'm not imagining things *AND* this has nothing to do with the fix for bug #152761 which has been backported to 4.1.2 for Fedora 9/10.
I digged a little into this problem. I added some debugging output to the functions: TerminalDisplay::calcGeometry() [called when TD receives a resizeEvent] TabbedViewContainerV2::setActiveView() [called when a new tab gets created] Setup: one window open, open new window, create new tab in first window: konsole(23390): Session::Session() konsole(23390): TVCV2::setActiveView() BEFORE setCurrentWidget() konsole(23390): TD::calcGeometry() BEGIN 1 x 1 ( QRect(0,0 595x462) ) konsole(23390): TD::calcGeometry() END 82 x 28 konsole(23390): TVCV2::setActiveView() BEFORE setCurrentIndex() konsole(23390): TVCV2::setActiveView() END -> OK, no resize Test step: resize second window, create new tab in first window: konsole(23390): Session::Session() konsole(23390): TVCV2::setActiveView() BEFORE setCurrentWidget() konsole(23390): TD::calcGeometry() BEGIN 1 x 1 ( QRect(0,0 595x462) ) konsole(23390): TD::calcGeometry() END 82 x 28 konsole(23390): TVCV2::setActiveView() BEFORE setCurrentIndex() konsole(23390): TD::calcGeometry() BEGIN 82 x 28 ( QRect(0,0 595x654) ) konsole(23390): TD::calcGeometry() END 82 x 40 konsole(23390): TVCV2::setActiveView() END -> first window resized to be the same size as second window So I think it is narrowed down to the call "_tabBar->setCurrentIndex(index)" that for some reason causes the window to resize. Not much yet, but maybe someone has a brilliant idea what causes this...
What I can't figure out: where does konsole store the information "this is the default window size"? That information gets obviously updated when you resize a window. A newly opened window will use this size, which is OK. Unfortunately the same information is also used when a new tab is added, i.e. the window with the new tab gets resized to the "default size". This information must be stored in some shared data structure, but where? As far as I have been able to figure out these are the shared data structures inside the konsole code: Application SessionManager Profile (if all tabs use the same profile) environment variable From my poking around I'm pretty sure that the information is not stored in Profile or an environment variable. Is this maybe caused by some "feature" of the base class KUniqueApplication? Anyone?
I might be wrong, but afaik KXmlGuiWindow is a subclass of KMainWindow, and KMainWindow supposedly saves the window size and position.
Hi Stefan, As Ivo mentioned, KMainWindow takes care of setting remembering the size of a Konsole window between closing it and opening it again. This is true of most KDE apps. The settings will be stored in the global Konsole settings file (~/.kde/share/config/konsolerc) The only special case is for the first time Konsole is started by a user, in which case it defaults to ~80 columns by ~40 rows in the default font. This size is set in ViewManager::createView(). This sets the size hint for the TerminalDisplay widget and Qt's layouting takes care of making the window the right size.
Had a quick dig. It looks like something changed in kdelibs recently. The backtrace which leads to the first window being resized, starting from the point where the new tab becomes active (and its SessionController emits a focused() signal) is: #13 0xb7dd8e9e in KMainWindow::restoreWindowSize (this=0x98b5c30, config=@0xbff854d4) at /home/robert/kde4/src/trunk/kdelibs/kdeui/widgets/kmainwindow.cpp:888 #14 0xb7dd8fcb in KMainWindow::applyMainWindowSettings (this=0x98b5c30, cg=@0xbff854d4, force=false) at /home/robert/kde4/src/trunk/kdelibs/kdeui/widgets/kmainwindow.cpp:711 #15 0xb7e15892 in KXmlGuiWindow::applyMainWindowSettings (this=0x98b5c30, config=@0xbff854d4, force=<value optimized out>) at /home/robert/kde4/src/trunk/kdelibs/kdeui/xmlgui/kxmlguiwindow.cpp:346 #16 0xb7e15a2d in KXmlGuiWindow::finalizeGUI (this=0x98b5c30) at /home/robert/kde4/src/trunk/kdelibs/kdeui/xmlgui/kxmlguiwindow.cpp:338 #17 0xb7e13c56 in KXMLGUIBuilder::finalizeGUI (this=0x98b5c48) at /home/robert/kde4/src/trunk/kdelibs/kdeui/xmlgui/kxmlguibuilder.cpp:409 #18 0xb7e15847 in KXmlGuiWindow::finalizeGUI (this=0x98b5c30, client=0x99bb58c) at /home/robert/kde4/src/trunk/kdelibs/kdeui/xmlgui/kxmlguiwindow.cpp:355 #19 0xb7e20bc0 in KXMLGUIFactory::addClient (this=0x9880560, client=0x99bb58c) at /home/robert/kde4/src/trunk/kdelibs/kdeui/xmlgui/kxmlguifactory.cpp:272 #20 0xb7f5102e in Konsole::MainWindow::activeViewChanged (this=0x98b5c30, controller=0x99bb578) at /home/robert/kde4/src/trunk/kdebase/apps/konsole/src/MainWindow.cpp:203 #21 0xb7f5122b in Konsole::MainWindow::qt_metacall (this=0x98b5c30, _c=QMetaObject::InvokeMetaMethod, ---Type <return> to continue, or q <return> to quit--- _id=9, _a=0xbff8575c) at /home/robert/kde4/build/trunk/kdebase/apps/konsole/src/MainWindow.moc:106 #22 0xb75f3cbb in QMetaObject::activate (sender=0x98720f0, from_signal_index=6, to_signal_index=6, argv=0xbff8575c) at kernel/qobject.cpp:3031 #23 0xb75f4245 in QMetaObject::activate (sender=0x98720f0, m=0xb7f66230, local_signal_index=2, argv=0xbff8575c) at kernel/qobject.cpp:3101 #24 0xb7f3e653 in Konsole::ViewManager::activeViewChanged (this=0x98720f0, _t1=0x99bb578) at /home/robert/kde4/build/trunk/kdebase/apps/konsole/src/ViewManager.moc:172 #25 0xb7f3f313 in Konsole::ViewManager::controllerChanged (this=0x98720f0, controller=0x99bb578) at /home/robert/kde4/src/trunk/kdebase/apps/konsole/src/ViewManager.cpp:522 #26 0xb7f4288f in Konsole::ViewManager::qt_metacall (this=0x98720f0, _c=QMetaObject::InvokeMetaMethod, _id=28, _a=0xbff858bc) at /home/robert/kde4/build/trunk/kdebase/apps/konsole/src/ViewManager.moc:147 #27 0xb75f3cbb in QMetaObject::activate (sender=0x99bb578, from_signal_index=9, to_signal_index=9, argv=0xbff858bc) at kernel/qobject.cpp:3031 #28 0xb75f4245 in QMetaObject::activate (sender=0x99bb578, m=0xb7f65870, local_signal_index=0, argv=0xbff858bc) at kernel/qobject.cpp:3101 #29 0xb7f17973 in Konsole::SessionController::focused (this=0x99bb578, _t1=0x99bb578) at /home/robert/kde4/build/trunk/kdebase/apps/konsole/src/SessionController.moc:185 #30 0xb7f1cc81 in Konsole::SessionController::eventFilter (this=0x99bb578, watched=0x99cd2a8, event=0xbff85d28) at /home/robert/kde4/src/trunk/kdebase/apps/konsole/src/SessionController.cpp:268
When I see this; #19 0xb7e20bc0 in KXMLGUIFactory::addClient (this=0x9880560, client=0x99bb58c) at /home/robert/kde4/src/trunk/kdelibs/kdeui/xmlgui/kxmlguifactory.cpp:272 #20 0xb7f5102e in Konsole::MainWindow::activeViewChanged (this=0x98b5c30, controller=0x99bb578) ¬ at /home/robert/kde4/src/trunk/kdebase/apps/konsole/src/MainWindow.cpp:203 I think I should point out that the API docs give me the distinct impression you are not suppost to call addClient for each tab. But instead call it once per main window.
Calling addClient to add the GUI of a given tab, is correct xmlgui usage. The reason for the applyMainWindowSettings call in finalizeGUI, added by ereslibre recently, is to handle hiding/showing toolbars when switching components (in konq, kontact etc.) correctly. Indeed it wasn't meant to resize windows, AFAIK. But applyMainWindowSettings resizes, by default (makes sense on startup, but not really later on). I think we need a new flag in KMainWindow::applyMainWindowSettings, or to split it up, so that kxmlguiwindow could say "apply toolbar settings", but not "and restore window size". Splitting up is better than a flag (especially for a virtual method...). Another (easier) solution might be to remember "I have restored window size already" and not do it twice for a given kmainwindow; ereslibre, do you see a use case where this might break?
I don't fully like the solution of adding another flag to applyMainWindowSettings (or overload the method with a new flag...). > Another (easier) solution might be to remember "I have restored window size > already" and not do it twice for a given kmainwindow; ereslibre, do you see a > use case where this might break? I think this solution would be nice, and I can't think at the moment of any use case that could break this behavior. When get home (if not yet fixed) I will give it a shot.
Created attachment 27872 [details] Restore window size only once OK, what about this change? Now konsole windows don't resize anymore when you add a new tab. Only problem I noticed is that for some reason the last resize is lost and konsolerc always contains the same size. So even if you change all windows to be a new size and then quit konsole, after restart it opens the windows with the previously saved window size.
Thanks for the patch Stefan. I cannot try it at the moment, I will be able in 6 hours more or less... However we usually use camelCase var names (isRestored instead of is_restored), and also I think you are missing the initialization of the var. I have no way of doing a diff here... but a rough idea I had to fix this was to do this check here: void KMainWindow::applyMainWindowSettings(const KConfigGroup &cg, bool force) { K_D(KMainWindow); kDebug(200) << "KMainWindow::applyMainWindowSettings " << cg.name(); QWidget *focusedWidget = QApplication::focusWidget(); d->letDirtySettings = false; if (!d->isRestored) { restoreWindowSize(cg); d->isRestored = true; } ... on kmainwindow.cpp file.
Yes ereslibre's way would be more correct; Stefan's patch would still resize the window many times if konsole was called with --geometry (ok, very unusual, but still; that's what d->care_about_geometry is about).
Created attachment 27880 [details] Restore window size only once Updated patch according to the comments. I wonder how care_about_geometry slipped through, though :-P But testing this simple fix revealed that session restore is completely broken too. I.e. if I restore my standard konsole session with two tabs I see this: konsole(15134): old window size QSize(700, 420) konsole(15134): new window size QSize(577, 415) -> initial window creation, resize to default in konsolerc. konsole(15134): old window size QSize(577, 415) konsole(15134): new window size QSize(582, 700) -> resize from session data. So we want this request to go through konsole(15134): old window size QSize(582, 700) konsole(15134): new window size QSize(577, 415) -> Add first tab. Resized back to konsolerc size :-( konsole(15134): old window size QSize(577, 415) konsole(15134): new window size QSize(577, 415) -> Add second tab. Resized back to konsolerc size :-( You of course only notice this when the size in konsolerc is different from the one in the session. I missed this fact when working on bug #152761 :-( With the patch applied you'll see only the first resize request, i.e. the windows will only be restored to the size stored in konsolerc. I'm wondering if we can check app->isSessionRestored() in applyMainWindowSettings and if we need a second flag for that.
Created attachment 27881 [details] Restore window size only once, but allow resize from session restore OK, I've added a second flag. Now session restore works correctly, i.e. a window will be resized twice.
I see what you mean, but the naming of the variables is really strange. If I start an application in KDE, getting d->isSessionRestored = true; is really strange. I think isRestored should be sizeApplied, and, hmm the logic around isSessionRestored needs some cleanup. Right now it's a hack that says "call restoreWindowSize twice", basically (once with isSessionRestored=false and once with isSessionRestored=true). But I think this breaks in apps using kparts, if the gui is built in two stages (app+part) before we get to the session management restoration. This could be cleaned up by using the fact that readPropertiesInternal is the one called by session management restoration. Yeah, I think it could work with a single bool sizeApplied, false initially, true after calling restoreWindowSize, and reset to false in readPropertiesInternal in order to force applyMainWindowSettings to apply the size again (because it comes from a different config file, that's the point...).
Created attachment 27885 [details] Only restore window size when needed I think this is the right patch... Stefan, have you got an SVN account ? If so, feel free to commit it. If not, I can do it giving you credits =)
Ahh I just had finished testing the exact same patch :-) At least I can confirm that the change works on KDE 4.1.2. I don't have a SVN account, so please go ahead and apply it. Please mention David too.
SVN commit 871409 by ereslibre: Initial patch by Stefan Becker (stefan.becker nokia com). David Faure gave good input on the process of the patch review. BUG: 172042 M +10 -1 kmainwindow.cpp M +1 -0 kmainwindow_p.h WebSVN link: http://websvn.kde.org/?view=rev&revision=871409
Could you commit it to the 4.1 branch? It would be a pain having to wait until 4.2 to get this fixed :)
FYI: I've created a backport request for Fedora 9: <https://bugzilla.redhat.com/show_bug.cgi?id=466979>
SVN commit 871465 by ereslibre: Backport: Initial patch by Stefan Becker (stefan.becker nokia com). David Faure gave good input on the process of the patch review. CCBUG: 172042 M +10 -1 kmainwindow.cpp M +1 -0 kmainwindow_p.h WebSVN link: http://websvn.kde.org/?view=rev&revision=871465