[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Re: [PATCH] for #87119 - kmail crashes after trying to
From:       Andreas Gungl <a.gungl () gmx ! de>
Date:       2004-09-01 20:19:45
Message-ID: 200409012219.45386 () gungl-dd ! de
[Download RAW message or body]

On Mittwoch 01 September 2004 00:27, Ingo Klöcker wrote:
> On Sunday 29 August 2004 23:34, Andreas Gungl wrote:
[...]
> > The patch can be immediately backported to the 3.3 branches. However
> > note, that in kmmainwidget.cpp (around line 3309) an if statement is
> > commented. To avoid a string change, I've coupled the plugging of
> > shortcut filters (AKA ad-hoc filters) into menu _and_ toolbar. The
> > next release could allow to separate this. But this means additional
> > changes in the GUI while the infrastructure is already there.
[...]

Thanks for the review. I knew you would find something, otherwise I wouldn't 
have asked. :-)

> One problem:
> - The "Apply Filter" entry in the context menu in the header pane is
> disabled.

Sorry, I didn't realize that. Patch for the 3.3 branch is attached, the 
change has already been committed to HEAD.

> One annoyance:
> - There's currently no way not to have the icons in the toolbar. Since
> we need a new setting for this in the filter configuration which we
> can't backport I don't really know whether we should backport this and
> thus force all users of KMail 1.7 to live with having all icons in the
> toolbar. But since it fixes some crashes it's probably worth
> backporting it anyway. A possible interim solution for the 3.3 branch
> would be to only add those filters to the toolbar for which the user
> has defined an icon.

The current behavior is intended, but not ideal. Do you mean with "for which 
the user has defined an icon" as not the default "gear" icon? That could 
work but could raise bug reports as well, because the user doesn't know 
about the underlaying logic.

> A minor nitpick: There should IMO be a line separator before the filter
> actions in the toolbar.

If I add a separator, then it's shown permanent (even in case no filter is 
plugged into the toolbar). I tried the "weakseparator" and hoped it would 
disappear if nothing else is plugged after it. But as we see, this doesn't 
work that way. Anybody else with detailed knowledge about the solution of 
the problem?

Regards,
Andreas

["kmmainwidget.cpp.diff" (text/x-diff)]

Index: kmmainwidget.cpp
===================================================================
RCS file: /home/kde/kdepim/kmail/kmmainwidget.cpp,v
retrieving revision 1.254
diff -u -3 -p -r1.254 kmmainwidget.cpp
--- kmmainwidget.cpp	31 Aug 2004 22:12:05 -0000	1.254
+++ kmmainwidget.cpp	1 Sep 2004 20:07:28 -0000
@@ -3284,6 +3284,7 @@ void KMMainWidget::initializeFilterActio
     mFilterTBarActions.clear();
   }
   if ( !mFilterMenuActions.isEmpty() ) {
+    mApplyFilterActionsMenu->popupMenu()->clear();
     if ( mGUIClient->factory() )
       mGUIClient->unplugActionList( "menu_filter_actions" );
     mFilterMenuActions.clear();
@@ -3305,6 +3306,7 @@ void KMMainWidget::initializeFilterActio
       filterAction = new KAction(as, icon, 0, filterCommand,
                                  SLOT(start()), actionCollection(),
                                  normalizedName.local8Bit());
+      filterAction->plug( mApplyFilterActionsMenu->popupMenu() );
       mFilterMenuActions.append(filterAction);
       // FIXME
       // uncomment the next if statement after the filter dialog supports


_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic