Bug 160407 - splitting konqueror main window may close the other views
Summary: splitting konqueror main window may close the other views
Status: RESOLVED FIXED
Alias: None
Product: konqueror
Classification: Applications
Component: general (show other bugs)
Version: SVN
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Konqueror Developers
URL:
Keywords:
: 176643 191553 201888 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-04-05 15:20 UTC by Christophe Marin
Modified: 2009-07-29 16:04 UTC (History)
9 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Animated gif, which shows the order from https://bugs.kde.org/show_bug.cgi?id=160407#c3 (553.91 KB, image/gif)
2008-08-06 12:05 UTC, Michal Vyskocil
Details
debugging (3.75 KB, patch)
2009-03-18 02:09 UTC, Marcel Partap
Details
Patch that I used for debugging (1.82 KB, patch)
2009-04-26 02:24 UTC, Frank Reininghaus
Details
Patch that fixes it for me (1.32 KB, patch)
2009-04-26 02:27 UTC, Frank Reininghaus
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Christophe Marin 2008-04-05 15:20:27 UTC
Version:           4.00.68 (KDE 4.0.68 >= 20080402) (using 4.00.68 (KDE 4.0.68 >= 20080402), compiled sources)
Compiler:          gcc
OS:                Linux (i686) release 2.6.24-12-generic

kdelibs/kdebase rev. 793803

Way to reproduce : 
- Split a konqueror window horizontally
- Click on the top view 
- Split it vertically
- The bottom view is closed and the main window is now split vertically.

The problem also appears if you split the bottom view horizontally then vertically.
Comment 1 S. Burmeister 2008-04-06 22:12:40 UTC
confirmed. Is this a feature?
Comment 2 Christophe Marin 2008-07-13 13:27:18 UTC
Still present in trunk (831648)
Comment 3 Michal Vyskocil 2008-08-06 12:04:06 UTC
The problem seems to be in an automatic hidding of the views. This way

1.) Split the main view vertically
2.) Split the rigth view horizontally
3.) Split the left view horiznotally
4.) Then find the three small vertical dots on the rigth side of the 
Konqueror's window and drag them to the left.

Shows the four indepentend views.
Comment 4 Michal Vyskocil 2008-08-06 12:05:25 UTC
Created attachment 26682 [details]
Animated gif, which shows the order from https://bugs.kde.org/show_bug.cgi?id=160407#c3
Comment 5 Felix 2008-08-27 01:08:38 UTC
I think this is a "feature". I also find it extremely annoying; it took me a while to figure out that the other views were indeed hiding rather than closing.

It only seems to happen when you try to split the first panel (top left).

It might be related to the Dolphin limit of only one vertical split, since it looks like Konqueror and Dolphin are using the same engine now.
Comment 6 Marcel Partap 2008-09-18 13:57:07 UTC
*noisy complaint* just wanted to file this aswell as:
'trying to further split a split view resplits the active view, removing the other split views'.. annoying indeed *g
Comment 7 Frank Reininghaus 2008-12-05 23:00:24 UTC
*** Bug 176643 has been marked as a duplicate of this bug. ***
Comment 8 Marcel Partap 2008-12-30 15:26:06 UTC
Ok tracking this further down, this only happens when you try to further split the initial view, subdividing works correctly for all other subviews...
Comment 9 Marcel Partap 2009-03-18 02:09:26 UTC
Created attachment 32218 [details]
debugging

i tried to nail the problem down but could not find the source of the anomaly... somehow the new parent container of the very first frame always resets the size of the other frame when messing with the it.. really out of clues here.
Comment 10 Thomas Savary 2009-04-05 19:04:48 UTC
Among Konqueror's present bugs, I find that this one is the most annoying. Although Dolphin has features that Konqueror does not have, the latter's ability to have multiple views makes it a much better file manager, in my opinion simply the best one of all time (because I need to be able to move a lot of files in temporay folders on several hard disks --- Dolphin's column view is great (I think that Konqueror should have it too, by the way), but not as flexible and convenient than Konqui's multiple split views... provided that they work correctly. I hope this bug will be fixed for KDE 4.3! I wish I were a coder myself to help. Alas, I'm not :-(
Comment 11 Frank Reininghaus 2009-04-26 02:24:56 UTC
Created attachment 33107 [details]
Patch that I used for debugging

When doing the second split, KonqFrameContainer::replaceChildFrame() saves the sizes of its two children (two frames, one of which is going to be split) and tries to restore these sizes after one frame has been replaced by a new KonqFrameContainer. As my debugging output shows, this does not work - the new container gets the size 0, and that seems to be the reason why the sizes get messed up later in some cases.

The reason is that the new KonqFrameContainer is not shown yet (this happens later in KonqViewManager::splitView()), and QSplitter::setSizes() ignores hidden widgets as can be seen in QSplitterPrivate::doResize().
Comment 12 Frank Reininghaus 2009-04-26 02:27:39 UTC
Created attachment 33108 [details]
Patch that fixes it for me

To fix it, the call to QSplitter::setSizes() must happen after the new KonqFrameContainer is shown in KonqViewManager::splitView(). To be able to do that, the old sizes must be saved in this method before the splitting starts.

I tried to make my patch fool-proof, but maybe it can be simplified a bit:

1. Maybe it's better not to introduce a new local variable 'parentKonqFrameContainer' in KonqViewManager::splitView(), but to change the type of 'parentContainer' from KonqFrameContainerBase* to KonqFrameContainer*, which requires a dynamic_cast. But I was not 100% sure if this cast is always possible - there must be some reason for having two classes KonqFrameContainerBase and KonqFrameContainer after all (but it's not obvious to me).

2. Maybe it's possible to remove the calls to QSplitter::sizes() and QSplitter::setSizes() in KonqFrameContainer::replaceChildFrame(). At least for the case where a KonqFrame is replaced by a KonqFrameContainer, they seem to be entirely irrelevant. But maybe that method can also be called in other contexts, I didn't check that.
Comment 13 Frank Reininghaus 2009-04-26 02:35:40 UTC
comment 10: Columns View is available in Konqueror.
Comment 14 David Faure 2009-04-27 01:26:17 UTC
Thanks for the investigation and patch!
To answer the last bit: the method is only called in one other context: removing a view that is inside a splitter. And I guess the code you mention is about splitter sizes of the grandparent, so the way to test this would be: split, split again, then remove a view. If the splitter sizes seem correct without the setSizes call, please remove it indeed; better not have redundant and unnecessary code, this stuff is confusing enough already ;)
Comment 15 Frank Reininghaus 2009-04-27 23:58:01 UTC
Concerning my remarks in comment 12:

1. That point is not valid because also KonqMainWindow and KonqFrameTabs inherit KonqFrameContainerBase.

2. Removing the calls is not possible - that would change the behaviour in the case pointed out by David.

I'll commit my patch in a minute...
Comment 16 Frank Reininghaus 2009-04-27 23:59:48 UTC
SVN commit 960084 by freininghaus:

Make sure that the sizes of other views are not changed when one view is split.

BUG: 160407


 M  +13 -0     konqviewmanager.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=960084
Comment 17 Frank Reininghaus 2009-04-28 00:02:20 UTC
SVN commit 960088 by freininghaus:

Make sure that the sizes of other views are not changed when one view is split.

Fix will be in KDE 4.2.3.

CCBUG: 160407


 M  +13 -0     konqviewmanager.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=960088
Comment 18 Marcel Partap 2009-04-29 16:51:27 UTC
Hey Frank THX! your fix works fine but you missed applying it to closure of the active view - in that case, the splitter sizes still jump to 100%/0%.. too busy to do it myself right now but guess you're more into it now anyways ;)
Comment 19 Frank Reininghaus 2009-04-29 17:12:26 UTC
Marcel, never forget the golden rule of bug reporting: Make sure you describe the problem in such a way that others can reproduce it ;-)

But anyway, after messing around a bit I found the following steps:

1. split top/bottom
2. split the active (i.e., bottom) view left/right
3. split the upper view top/bottom
4. close the active view (i.e., second view from top).

In that case, the top view is indeed resized to 100%, I assume that's the problem you mean? I'll try to have a look, but I can't promise I can fix it before 4.2.3 tagging (i.e., today).
Comment 20 Frank Reininghaus 2009-04-29 17:20:29 UTC
OK, it's actually easier to reproduce: 

1. split top/bottom
2. split the upper view in any way
3. close one of the subviews of the upper view

The same works with left/right splits. At least this problem is present in 4.2.2 as well (-> it's not a regression).
Comment 21 Frank Reininghaus 2009-04-29 18:40:33 UTC
The fix for the new issue is basically copying&pasting the code I added to KonqViewManager::splitView() to KonqViewManager::removeView(). The reason why it failed in the first place is a bit different from what I wrote in comment 11 for the original issue though: the setSizes() call in KonqFrameContainer::replaceChildFrame() works here (because no widgets are hidden), but not quite as intended: the problem is that the splitter temporarily contains 3 widgets after insertChildFrame() is called: the KonqFrameContainer which is to be removed is still there (and shifted one position to the back in the list of the splitter's children). It's removed from the splitter only when it gets deleted.

If the container was the second child of the grandparent of the removed view, this does not matter: it's the third child now, so it does not interfere with the setSizes() call which only affects the first two. However, if the container was the first child of the grandparent, it is the second now, so the bottom view is the third child and gets the size 0 after the setSizes() call.

I'll commit a fix in a minute. I hope all issues causing unexpected resizing of views are resolved now :-)
Comment 22 Frank Reininghaus 2009-04-29 18:43:15 UTC
SVN commit 961199 by freininghaus:

Fix an issue which is similar to bug 160407: Splitting the first child of a KonqFrameContainer and then removing one of the grandchildren also resized the bottom child to zero size.

CCBUG: 160407


 M  +13 -0     konqviewmanager.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=961199
Comment 23 Frank Reininghaus 2009-04-29 18:51:50 UTC
SVN commit 961201 by freininghaus:

Fix an issue which is similar to bug 160407: Splitting the first child of a KonqFrameContainer and then removing one of the grandchildren also resized the bottom child to zero size.

Fix will be in KDE 4.2.3.

CCBUG: 160407


 M  +13 -0     konqviewmanager.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=961201
Comment 24 Marcel Partap 2009-04-29 18:57:14 UTC
G0od bowling Frank! Kudos!
Comment 25 Frank Reininghaus 2009-05-04 19:14:16 UTC
*** Bug 191553 has been marked as a duplicate of this bug. ***
Comment 26 Frank Reininghaus 2009-07-29 16:04:19 UTC
*** Bug 201888 has been marked as a duplicate of this bug. ***