Bug 144958

Summary: kalarm doesn't send attachements with scheduled e-mails
Product: [Applications] kmail Reporter: Stephan Buehne <sbuehne>
Component: sendingAssignee: kdepim bugs <kdepim-bugs>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: openSUSE   
OS: Linux   
Latest Commit: Version Fixed In:

Description Stephan Buehne 2007-05-02 15:44:05 UTC
Version:           1.4.10 (using KDE KDE 3.5.6)
Installed from:    SuSE RPMs
OS:                Linux

This bug was previously reported in Bug# 118286. In this Bug report a fix was announced for KDE 3.5.1.
Is kalarm in the recent version (3.5.6) once again broken?
Comment 1 David Jarvie 2007-05-05 15:01:41 UTC
This is actually a bug in kmail. A workaround for the problem, if you have sendmail set up on your system, would be to select sendmail as the mail delivery method in KAlarm's Preferences dialog.

The KMail bug occurs when MailServiceImpl::sendMessage(..., KURL::List) is called by DCOP. The sequence of events then is:

MailServiceImpl::sendMessage():
  cWin->addAttachment( *itr, "" );
  KMComposeWin::addAttachment(KURL,QString):
    addAttach(url);
    KMComposeWin::addAttach(const KURL):
This method calls a KIO job and then returns immediately. The problem is that as soon as control returns to MailServiceImpl::sendMessage(), it calls KMComposeWin::send(1); which immediately attempts to send the message. But the KIO job still has not completed, so the attachment has not yet been added to KMComposeWin::mAtmList, and the message is sent without the attachment.
    
Comment 2 David Jarvie 2007-05-11 00:55:05 UTC
SVN commit 663344 by djarvie:

Fix sendMessage() DCOP call failing to wait for attachments to be downloaded
and included in the message, before sending it.

This fixes KAlarm failing to send attachments when KMail is used as the email
client. The fix will be included in KDE 3.5.7.

BUG: 144958


 M  +2 -1      composer.h  
 M  +56 -2     kmcomposewin.cpp  
 M  +7 -1      kmcomposewin.h  
 M  +1 -6      mailserviceimpl.cpp  


--- branches/KDE/3.5/kdepim/kmail/composer.h #663343:663344
@@ -42,6 +42,7 @@
      * From MailComposerIface
      */
     virtual void send( int how ) = 0;
+    virtual void addAttachmentsAndSend(const KURL::List &urls, const QString &comment, int how) = 0;
     virtual void addAttachment( KURL url, QString comment ) = 0;
     virtual void addAttachment( const QString & name,
                                 const QCString & cte,
@@ -136,7 +137,7 @@
     virtual void autoSaveMessage() = 0;
 
   public: // kmkernel, attachmentlistview
-    virtual void addAttach( const KURL url ) = 0;
+    virtual bool addAttach( const KURL url ) = 0;
 
   public: // kmcommand
     /**
--- branches/KDE/3.5/kdepim/kmail/kmcomposewin.cpp #663343:663344
@@ -461,6 +461,41 @@
 }
 
 //-----------------------------------------------------------------------------
+void KMComposeWin::addAttachmentsAndSend(const KURL::List &urls, const QString &/*comment*/, int how)
+{
+  if (urls.isEmpty())
+  {
+    send(how);
+    return;
+  }
+  mAttachFilesSend = how;
+  mAttachFilesPending = urls;
+  connect(this, SIGNAL(attachmentAdded(const KURL&, bool)), SLOT(slotAttachedFile(const KURL&)));
+  for( KURL::List::ConstIterator itr = urls.begin(); itr != urls.end(); ++itr ) {
+    if (!addAttach( *itr ))
+      mAttachFilesPending.remove(mAttachFilesPending.find(*itr)); // only remove one copy of the url
+  }
+
+  if (mAttachFilesPending.isEmpty() && mAttachFilesSend == how)
+  {
+    send(mAttachFilesSend);
+    mAttachFilesSend = -1;
+  }
+}
+
+void KMComposeWin::slotAttachedFile(const KURL &url)
+{
+  if (mAttachFilesPending.isEmpty())
+    return;
+  mAttachFilesPending.remove(mAttachFilesPending.find(url)); // only remove one copy of url
+  if (mAttachFilesPending.isEmpty())
+  {
+    send(mAttachFilesSend);
+    mAttachFilesSend = -1;
+  }
+}
+
+//-----------------------------------------------------------------------------
 void KMComposeWin::addAttachment(KURL url,QString /*comment*/)
 {
   addAttach(url);
@@ -2220,13 +2255,13 @@
 }
 
 //-----------------------------------------------------------------------------
-void KMComposeWin::addAttach(const KURL aUrl)
+bool KMComposeWin::addAttach(const KURL aUrl)
 {
   if ( !aUrl.isValid() ) {
     KMessageBox::sorry( this, i18n( "<qt><p>KMail could not recognize the location of the attachment (%1);</p>"
                                  "<p>you have to specify the full path if you wish to attach a file.</p></qt>" )
                         .arg( aUrl.prettyURL() ) );
-    return;
+    return false;
   }
   KIO::TransferJob *job = KIO::get(aUrl);
   KIO::Scheduler::scheduleJob( job );
@@ -2238,10 +2273,12 @@
     ld.encoding = aUrl.fileEncoding().latin1();
 
   mMapAtmLoadData.insert(job, ld);
+  mAttachJobs[job] = aUrl;
   connect(job, SIGNAL(result(KIO::Job *)),
           this, SLOT(slotAttachFileResult(KIO::Job *)));
   connect(job, SIGNAL(data(KIO::Job *, const QByteArray &)),
           this, SLOT(slotAttachFileData(KIO::Job *, const QByteArray &)));
+  return true;
 }
 
 
@@ -2588,10 +2625,20 @@
 {
   QMap<KIO::Job*, atmLoadData>::Iterator it = mMapAtmLoadData.find(job);
   assert(it != mMapAtmLoadData.end());
+  KURL attachURL;
+  QMap<KIO::Job*, KURL>::iterator jit = mAttachJobs.find(job);
+  bool attachURLfound = (jit != mAttachJobs.end());
+  if (attachURLfound)
+  {
+    attachURL = jit.data();
+    mAttachJobs.remove(jit);
+  }
   if (job->error())
   {
     mMapAtmLoadData.remove(it);
     job->showErrorDialog();
+    if (attachURLfound)
+      emit attachmentAdded(attachURL, false);
     return;
   }
   if ((*it).insert)
@@ -2603,6 +2650,8 @@
     else
       mEditor->insert( QString::fromLocal8Bit( (*it).data ) );
     mMapAtmLoadData.remove(it);
+    if (attachURLfound)
+      emit attachmentAdded(attachURL, true);
     return;
   }
   const QCString partCharset = (*it).url.fileEncoding().isEmpty()
@@ -2697,6 +2746,8 @@
     if (!dlg.exec()) {
       delete msgPart;
       msgPart = 0;
+      if (attachURLfound)
+        emit attachmentAdded(attachURL, false);
       return;
     }
   }
@@ -2705,6 +2756,9 @@
 
   // add the new attachment to the list
   addAttach(msgPart);
+
+  if (attachURLfound)
+    emit attachmentAdded(attachURL, true);
 }
 
 
--- branches/KDE/3.5/kdepim/kmail/kmcomposewin.h #663343:663344
@@ -111,6 +111,7 @@
    * From MailComposerIface
    */
   void send(int how);
+  void addAttachmentsAndSend(const KURL::List &urls, const QString &comment, int how);
   void addAttachment(KURL url,QString comment);
   void addAttachment(const QString &name,
                     const QCString &cte,
@@ -253,6 +254,7 @@
   void slotPrint();
   void slotAttachFile();
   void slotInsertRecentFile(const KURL&);
+  void slotAttachedFile(const KURL&);
 public slots: // kmkernel, callback
   void slotSendNow();
 private slots:
@@ -454,7 +456,7 @@
   void alignmentChanged( int );
 
 public: // kmkernel, attachmentlistview
-  void addAttach(const KURL url);
+  bool addAttach(const KURL url);
 
 public: // kmcommand
   /**
@@ -470,6 +472,7 @@
 
 signals:
   void applyChangesDone( bool );
+  void attachmentAdded( const KURL&, bool success );
 
 private:
   /**
@@ -751,6 +754,9 @@
 
   QStringList mFolderNames;
   QValueList<QGuardedPtr<KMFolder> > mFolderList;
+  QMap<KIO::Job*, KURL> mAttachJobs;
+  KURL::List mAttachFilesPending;
+  int mAttachFilesSend;
 
 private:
   // helper method for slotInsert(My)PublicKey()
--- branches/KDE/3.5/kdepim/kmail/mailserviceimpl.cpp #663343:663344
@@ -75,12 +75,7 @@
   KMail::Composer * cWin = KMail::makeComposer( msg );
   cWin->setCharset("", TRUE);
 
-  for( KURL::List::ConstIterator itr = attachments.begin();
-       itr != attachments.end(); ++itr ) {
-    cWin->addAttachment( *itr, "" );
-  }
-
-  cWin->send( 1 );//send now
+  cWin->addAttachmentsAndSend(attachments, "", 1);//send now
   return true;
 }