Bug 100280

Created attachment 9840 [details]
patch

Why single out Kopete and not all the other apps with KABC integration? I mean, there is also Konversation, KOrganizer, KAlarmd... any/all of these seem to connect to download a remote resource I have (a remote VCard on a web server) when I start them, so I assume they would do the same for IMAP resources.

I don't think this is the way to go... why can't the DCOP code in the IMAP resource just check if KMail is already running first, and *not* start KMail if it is not already running, but instead present a warning or error dialog?

Adding the ability to disable KABC integration for everything else because some people use IMAP resources with a bug seems ridiculous... correct the problem in the IMAP resouce, not everywhere else.

On Friday 25 February 2005 18:11, Jason Keirstead wrote:
> ------- Why single out Kopete and not all the other apps with KABC
> integration? I mean, there is also Konversation, KOrganizer, KAlarmd...
> any/all of these seem to connect to download a remote resource I have (a
> remote VCard on a web server) when I start them, so I assume they would do
> the same for IMAP resources.

  I'm fixing all of the ones I use and filing bugs everywhere.

> I don't think this is the way to go... why can't the DCOP code in the IMAP
> resource just check if KMail is already running first, and *not* start
> KMail if it is not already running, but instead present a warning or error
> dialog?

  It does.  It's a KUniqueApp.  It's still very slow.

> Adding the ability to disable KABC integration for everything else because
> some people use IMAP resources with a bug seems ridiculous... correct the
> problem in the IMAP resouce, not everywhere else.

  It won't be fixed there according to KMail developers.  At least not anytime 
soon.

Oh I see the problem. KDcopServiceStarter doesn't let you just *query* to see if a service is running, it always freaking starts it.

Seems like all that is needed, is to add a new flag to be passed to this method, which will not start the service if it is not already running.

The resource is already checking for this case, so it would need no changes there either:

int result = KDCOPServiceStarter::self()->
      findServiceFor( "DCOP/ResourceBackend/IMAP", QString::null,
                      QString::null, &error, &dcopService );
    if ( result != 0 ) {
      kdError(5650) << "Couldn't connect to the IMAP resource backend\n";
      // TODO: You might want to show "error" (if not empty) here,
      // using e.g. KMessageBox
      return false;
    }


And findServiceFor already has a flags paramater... currently it is unused, but we could easily make use of it here.

Proposed patch:

Index: kresources/kolab/shared/kmailconnection.cpp
===================================================================
RCS file: /home/kde/kdepim/kresources/kolab/shared/kmailconnection.cpp,v
retrieving revision 1.8
diff -u -p -w -B -r1.8 kmailconnection.cpp
--- kresources/kolab/shared/kmailconnection.cpp 19 Jan 2005 15:18:14 -0000      1.8
+++ kresources/kolab/shared/kmailconnection.cpp 25 Feb 2005 23:34:27 -0000
@@ -77,7 +77,7 @@ bool KMailConnection::connectToKMail()
     QCString dcopService;
     int result = KDCOPServiceStarter::self()->
       findServiceFor( "DCOP/ResourceBackend/IMAP", QString::null,
-                      QString::null, &error, &dcopService );
+                      QString::null, &error, &dcopService, KDCOPServiceStarter::NoAutoStart );
     if ( result != 0 ) {
       kdError(5650) << "Couldn't connect to the IMAP resource backend\n";
       // TODO: You might want to show "error" (if not empty) here,


Index: kio/kio/kdcopservicestarter.cpp
===================================================================
RCS file: /home/kde/kdelibs/kio/kio/kdcopservicestarter.cpp,v
retrieving revision 1.1
diff -u -p -w -B -r1.1 kdcopservicestarter.cpp
--- kio/kio/kdcopservicestarter.cpp     4 Jan 2003 21:52:23 -0000       1.1
+++ kio/kio/kdcopservicestarter.cpp     25 Feb 2005 23:34:41 -0000
@@ -68,7 +68,7 @@ int KDCOPServiceStarter::findServiceFor(
     KService::Ptr ptr = offers.first();
     QCString dcopService = ptr->property("X-DCOP-ServiceName").toString().latin1();

-    if ( !kapp->dcopClient()->isApplicationRegistered( dcopService ) )
+    if ( !kapp->dcopClient()->isApplicationRegistered( dcopService ) && !(flags & NoAutoStart) )
     {
         QString error;
         if ( startServiceFor( serviceType, constraint, preferences, &error, &dcopService, flags ) != 0 )
Index: kio/kio/kdcopservicestarter.h
===================================================================
RCS file: /home/kde/kdelibs/kio/kio/kdcopservicestarter.h,v
retrieving revision 1.4
diff -u -p -w -B -r1.4 kdcopservicestarter.h
--- kio/kio/kdcopservicestarter.h       7 Sep 2004 12:51:09 -0000       1.4
+++ kio/kio/kdcopservicestarter.h       25 Feb 2005 23:34:42 -0000
@@ -38,6 +38,14 @@ public:
     static KDCOPServiceStarter* self();

     /**
+     * Flags accepted by findServiceFor
+     */
+    enum FindServiceFlags
+    {
+        NoAutoStart = 1
+    };
+
+    /**
      * Check if a given DCOP interface is available - from the serviceType it's supposed to implement.
      *
      * The trader is queried to find the preferred application for this serviceType,
@@ -54,7 +62,7 @@ public:
      * @param dcopService On success, @p dcopService contains the DCOP name
      *         under which this service is available. If the pointer is 0 the argument
      *         will be ignored
-     * @param flags for future extensions (currently unused)
+     * @param flags for future extensions
      *
      * @return an error code indicating success (== 0) or failure (> 0).
      */
~
~

On Friday 25 February 2005 18:27, Jason Keirstead wrote:
> ------- Additional Comments From jason keirstead org  2005-02-26 00:27
> ------- Oh I see the problem. KDcopServiceStarter doesn't let you just
> *query* to see if a service is running, it always freaking starts it.
>
> Seems like all that is needed, is to add a new flag to be passed to this
> method, which will not start the service if it is not already running.
>
> The resource is already checking for this case, so it would need no changes
> there either:
>
> int result = KDCOPServiceStarter::self()->
>       findServiceFor( "DCOP/ResourceBackend/IMAP", QString::null,
>                       QString::null, &error, &dcopService );
>     if ( result != 0 ) {
>       kdError(5650) << "Couldn't connect to the IMAP resource backend\n";
>       // TODO: You might want to show "error" (if not empty) here,
>       // using e.g. KMessageBox
>       return false;
>     }
>
>
> And findServiceFor already has a flags paramater... currently it is unused,
> but we could easily make use of it here.

  Nice, I'll give it a try to see what the results are now.  Might take until 
tomorrow or so.

Please replace my above KIO patch with this one. The above had a problem.

Index: kio/kio/kdcopservicestarter.cpp
===================================================================
RCS file: /home/kde/kdelibs/kio/kio/kdcopservicestarter.cpp,v
retrieving revision 1.1
diff -u -p -w -B -r1.1 kdcopservicestarter.cpp
--- kio/kio/kdcopservicestarter.cpp     4 Jan 2003 21:52:23 -0000       1.1
+++ kio/kio/kdcopservicestarter.cpp     26 Feb 2005 02:41:56 -0000
@@ -70,6 +71,12 @@ int KDCOPServiceStarter::findServiceFor(

     if ( !kapp->dcopClient()->isApplicationRegistered( dcopService ) )
     {
+        if( flags & NoAutoStart )
+        {
+            //Do not automatically start app
+            return -3;
+        }
+
         QString error;
         if ( startServiceFor( serviceType, constraint, preferences, &error, &dcopService, flags ) != 0 )
         {
Index: kio/kio/kdcopservicestarter.h
===================================================================
RCS file: /home/kde/kdelibs/kio/kio/kdcopservicestarter.h,v
retrieving revision 1.4
diff -u -p -w -B -r1.4 kdcopservicestarter.h
--- kio/kio/kdcopservicestarter.h       7 Sep 2004 12:51:09 -0000       1.4
+++ kio/kio/kdcopservicestarter.h       26 Feb 2005 02:41:57 -0000
@@ -38,6 +38,14 @@ public:
     static KDCOPServiceStarter* self();

     /**
+     * Flags accepted by findServiceFor
+     */
+    enum FindServiceFlags
+    {
+        NoAutoStart = 1
+    };
+
+    /**
      * Check if a given DCOP interface is available - from the serviceType it's supposed to implement.
      *
      * The trader is queried to find the preferred application for this serviceType,
@@ -54,7 +62,7 @@ public:
      * @param dcopService On success, @p dcopService contains the DCOP name
      *         under which this service is available. If the pointer is 0 the argument
      *         will be ignored
-     * @param flags for future extensions (currently unused)
+     * @param flags for future extensions
      *
      * @return an error code indicating success (== 0) or failure (> 0).
      */

Can this be reassigned to kdepim or kmail?

> Can this be reassigned to kdepim or kmail?


   I don't think KMail is going to "fix" it on that side any time soon, but it 
has to be fixed somewhere, so either KMail and all the resources stuff gets 
redesigned, or the apps that trigger it fix the problem there...  

My above patch fixes the problem fine for me.

Someone with more KIO / KDEPIM knowledge just needs to review it and commit....

I can produce this problem too.  Is the patch to be included in KDE 3.4.1?  This is fairly irritating but quite liveable in the short term.

*** Bug 106918 has been marked as a duplicate of this bug. ***

*** This bug has been marked as a duplicate of 97764 ***

*** Bug 137442 has been marked as a duplicate of this bug. ***