Bug 153646 - Kate crashes when you exit the built-in shell
Summary: Kate crashes when you exit the built-in shell
Status: RESOLVED FIXED
Alias: None
Product: kate
Classification: Applications
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR crash
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-07 22:11 UTC by Ivo Anjo
Modified: 2007-12-10 23:54 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
backtrace (11.33 KB, text/plain)
2007-12-07 22:12 UTC, Ivo Anjo
Details
Patch for konsolepart (852 bytes, patch)
2007-12-10 14:07 UTC, Thomas Friedrichsmeier
Details
Different approach (2.29 KB, patch)
2007-12-10 14:59 UTC, Thomas Friedrichsmeier
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ivo Anjo 2007-12-07 22:11:54 UTC
Version:            (using KDE Devel)
Installed from:    Compiled sources
OS:                Linux

Steps to reproduce:
- open kate
- click terminal button on the bottom
- type "exit" (sometimes it doesn't crash the first or second times, but it eventually does)
Comment 1 Ivo Anjo 2007-12-07 22:12:31 UTC
Created attachment 22407 [details]
backtrace
Comment 2 Thomas Friedrichsmeier 2007-12-10 14:07:46 UTC
Created attachment 22444 [details]
Patch for konsolepart
Comment 3 Thomas Friedrichsmeier 2007-12-10 14:19:25 UTC
Ok, the real messup is in konsolepart. This bug should be reassigned there. The problem is, that it's not a good idea to forcibly insert and remove child XMLGUIClient, though I don't have a good understanding, where the problem is. Side-effects other than the crash include that the "Scrollback" menu is shown as "No text", otherwise.

On a more general scope,
a) I don't quite understand, why clients need to be inserted / removed at all, when the konsole view changes. Can't the same XMLGUIClient be reused?
b) I doubt it's a good idea to forcibly insert / remove the client in the hosting application. At least, probably this should not happen *unless* there already is a factory() for the part, i.e. the hosting application has actively merged the part GUI. The part should not try to find a factory by all means. But this is the reason why I'm not re-assigning, yet:

The current behavior adds the konsolepart GUI to kate, automatically. This is certainly desirable at least for the konsole context menu. However, I'm not sure, whether we really want all the other menu-clutter that the konsolepart adds. Comments?
Comment 4 Thomas Friedrichsmeier 2007-12-10 14:59:55 UTC
Created attachment 22448 [details]
Different approach

Or, in other words, perhaps this second patch is more desirable (it has it's
own ugliness, but removes other ugliness):

Here, if there is no factory for creating the context menu, one will be created
on the fly. The good thing is that this does not force the hosting application
to merge the full GUI just to get the context menu.
Comment 5 Anders Lund 2007-12-10 20:11:35 UTC
Thomas, did you do anything to drag the konsole maintainers attention to this?
Comment 6 Robert Knight 2007-12-10 21:09:04 UTC
> The problem is, that it's not a good idea to forcibly insert
>  and remove child XMLGUIClient, though I don't have a good
>  understanding, where the problem is. 

Each terminal session in the Part has its own client which provides session-specific actions.

The only menu needed in the part is the context menu on right-click, as per KDE 3.  In KDE 3 the context menu for the part was constructed manually in code, I wanted to avoid this if possible and use XMLGUI instead.  There isn't supposed to be any integration of Konsole's main menu and the hosting application's menu.

> However, I'm not sure, whether we really want 
> all the other menu-clutter that the konsolepart adds

I'm pretty sure you don't want that.  Any menu items which are usually on the main menu in the main Konsole application which are needed from the part should ideally be put into the context menu.

I cannot test the attached patch as my laptop is without a hard drive at the moment so I am working off a live CD.  The second one looks correct.  
Comment 7 Thomas Friedrichsmeier 2007-12-10 22:40:03 UTC
Ok, should I apply it then, or would you rather take care of things yourself from here on (get well, soon, poor laptop)?
Comment 8 Robert Knight 2007-12-10 23:14:27 UTC
> Ok, should I apply it then, 

Yes please.
Comment 9 Thomas Friedrichsmeier 2007-12-10 23:54:44 UTC
SVN commit 747059 by tfry:

Create a KXMLGUIFactory for the context menu on the fly, if needed, instead of forcibly merging with the hosts GUI.
BUG: 153646

 M  +2 -22     Part.cpp  
 M  +11 -0     SessionController.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=747059