Bug 113458 - Wrong checkbox in the layout when adding attachments to the composer
Summary: Wrong checkbox in the layout when adding attachments to the composer
Status: RESOLVED FIXED
Alias: None
Product: kmail
Classification: Unmaintained
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: kdepim bugs
URL:
Keywords:
: 125742 (view as bug list)
Depends on:
Blocks:
 
Reported: 2005-09-27 20:24 UTC by Antonio E.
Modified: 2007-09-14 12:17 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Screenshot with the bug (42.54 KB, image/jpeg)
2005-09-27 20:26 UTC, Antonio E.
Details
Patch which fixes the problem (7.61 KB, patch)
2006-01-15 02:58 UTC, Thomas McGuire
Details
New screenshot with the bug (38.47 KB, image/png)
2006-01-15 12:29 UTC, Antonio E.
Details
New patch (12.70 KB, patch)
2006-02-01 03:38 UTC, Thomas McGuire
Details
Trunk version of the patch (12.60 KB, patch)
2006-11-06 00:47 UTC, Thomas McGuire
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Antonio E. 2005-09-27 20:24:48 UTC
Version:           kde-3.5_beta1 (using KDE Devel)
Installed from:    Compiled sources
Compiler:          gcc-3.3.6 
OS:                Linux

I've found an incorrect placed checkbox in the attachment box.

I opened Kontact, and selected Kmail from there. Then I used the composer to create a new email. I dragged some jpg files there. The files were successfully imported except that a strange checkbox appeared un the first row of the imported files table.

Using the scrollbar did it to disapear.

I'm going to attach a screenshot.
Comment 1 Antonio E. 2005-09-27 20:26:28 UTC
Created attachment 12734 [details]
Screenshot with the bug

In the screenshot we can see the buggy checkbox, it's in the bottom left, in
the first row in the column called Nombre.
Comment 2 Antonio E. 2005-09-27 20:31:18 UTC
After this, opening an email with attachment I got the same problem.
Comment 3 Antonio E. 2005-09-27 20:33:18 UTC
Additional comment, this happened dragging several files at the same time.
Comment 4 Antonio E. 2005-10-21 01:48:41 UTC
This problem continues happening in kde-3.5_beta2
Comment 5 Thomas McGuire 2006-01-15 02:58:17 UTC
Created attachment 14257 [details]
Patch which fixes the problem

The problem is that the checkboxes are initalized in the constructor of the
list view item, but do not yet have the correct position. The correct position
is calculated in paintCell(). However, paintCell() is not called for
attachments
which are off-screen, which results in checkboxes drawn at 0,0 because
their position is never calculated.

The patch solves the problem by hiding all checkboxes until they are to be
drawn
in paintCell(). Also some code duplication is removed.
Additionally, some outdated include files are removed, which are apparently the

leftovers of a split of one large files into several smaller ones.
IMHO the files for the KMAtmListView and KMAtmListViewItem classes should be
renamed to match their class names as well.

Please review the patch and commit to 3.5 and trunk if everything seems
correct.
Comment 6 Antonio E. 2006-01-15 12:29:49 UTC
Created attachment 14258 [details]
New screenshot with the bug

Thanks for work on this.

I've recompiled applying the patch (gcc with ccache). 
It seems to be better, but sometimes I get again the problem. It's less
frequently, but it still happens. Now it doesn't happen in the first column,
but it does in last three ones as you can see in the attached screenshot
(column names: Comprimir, Cifrar, Firma (in english would be something like:
compress, cipher, sign).
Comment 7 Thomas McGuire 2006-02-01 03:38:05 UTC
Created attachment 14473 [details]
New patch

Sorry for the late answer, I was lazy the last weeks.
Thanks for testing the patch.

The new version of the patch is a big rewrite, and I think solves all problems.

If you still find problems, please give a few more details, "happens sometimes"

is not enough.
In your first comment, the reason was adding many attachments at once, in fact
so many that some could not be shown on screen.
In your second comment I *think* the reason was you clicked the column headers
to sort, but I'm not sure.

The patch also includes the fix for bug #119526, I did not dare to edit the
diff file to exclude to patch. In fact I think my first patch
was corrupted because of this.

I noticed another problem while testing: Strange redraw glitches at the
selected attachment during resizing the column header. This behaviour existed
in the original version as well, I couldn't solve it.
Comment 8 Tom Albers 2006-10-28 13:47:19 UTC
Why is the patch that big? Is it still valid for the current KMail?
Comment 9 Thomas McGuire 2006-10-28 15:17:40 UTC
I just checked with KMail from KDE 3.5.5, the bug is still there.

I will check against the 3.5.5 branch this weekend and update my patch if it is not valid anymore.

The patch is that big for several reasons:
- It removes superflous #includes from an apparant spilt between kmcomposewin.cpp and kmatmlistview.cpp
- It includes the patch for bug #119526 (also still valid)
- The patch itself is a bit tricky (I could not really find a clean solutions, see my comment in the last patch) and has a large comment
Comment 10 Thomas McGuire 2006-10-29 19:24:19 UTC
The patch still cleanly applies and everything seems to work (there were no changes in kmatmlistview.cpp since I submitted the patch).

Tom, will you commit this patch or is there anything wrong with it?
Comment 11 Thomas McGuire 2006-11-06 00:47:13 UTC
Created attachment 18430 [details]
Trunk version of the patch

This patch is against the trunk version of KMail.
Comment 12 Allen Winter 2006-11-06 19:19:47 UTC
SVN commit 602743 by winterz:

Patch from Thomas McGuire that fixes the bugs:
+ Wrong checkbox in the layout when adding attachments to the composer
+ attachment display in editor: "sort by size" sorts alphanumerically, not by size

Thanks for the patch Thomas and sorry it took so long.
Will merge into the 3.5 branch shortly.

CCBUGS: 113458, 119526


 M  +104 -215  branches/work/kdepim-3.5.5+/kmail/kmatmlistview.cpp  
 M  +31 -22    branches/work/kdepim-3.5.5+/kmail/kmatmlistview.h  
 M  +1 -0      branches/work/kdepim-3.5.5+/kmail/kmcomposewin.cpp  


--- branches/work/kdepim-3.5.5+/kmail/kmatmlistview.cpp #602742:602743
@@ -1,147 +1,30 @@
 // -*- mode: C++; c-file-style: "gnu" -*-
-// kmcomposewin.cpp
+// kmatmlistview.cpp
 // Author: Markus Wuebben <markus.wuebben@kde.org>
 // This code is published under the GPL.
 
 #include <config.h>
 
 #include "kmatmlistview.h"
-
-#include "kmmainwin.h"
-#include "kmreadermainwin.h"
-#include "messagesender.h"
-#include "kmmsgpartdlg.h"
-#include <kpgpblock.h>
-#include <kaddrbook.h>
-#include "kmaddrbook.h"
-#include "kmmsgdict.h"
-#include "kmfolderimap.h"
-#include "kmfoldermgr.h"
-#include "kmfoldercombobox.h"
-#include "kmtransport.h"
-#include "kmcommands.h"
-#include "kcursorsaver.h"
-#include "partNode.h"
-#include "attachmentlistview.h"
-#include "transportmanager.h"
-using KMail::AttachmentListView;
-#include "dictionarycombobox.h"
-using KMail::DictionaryComboBox;
-#include "addressesdialog.h"
-using KPIM::AddressesDialog;
-#include "addresseeemailselection.h"
-using KPIM::AddresseeEmailSelection;
-using KPIM::AddresseeSelectorDialog;
-#include <maillistdrag.h>
-using KPIM::MailListDrag;
-#include "recentaddresses.h"
-using KRecentAddress::RecentAddresses;
-#include "kleo_util.h"
-#include "stl_util.h"
-#include "recipientseditor.h"
-
-#include "attachmentcollector.h"
-#include "objecttreeparser.h"
-
-#include "kmfoldermaildir.h"
-
-#include <libkpimidentities/identitymanager.h>
-#include <libkpimidentities/identitycombo.h>
-#include <libkpimidentities/identity.h>
-#include <libkdepim/kfileio.h>
-#include <libemailfunctions/email.h>
-#include <kleo/cryptobackendfactory.h>
-#include <kleo/exportjob.h>
-#include <ui/progressdialog.h>
-#include <ui/keyselectiondialog.h>
-
-#include <gpgmepp/context.h>
-#include <gpgmepp/key.h>
-
-#include <kabc/vcardconverter.h>
-#include <libkdepim/kvcarddrag.h>
-#include <kio/netaccess.h>
-
-
-#include "klistboxdialog.h"
-
-#include "messagecomposer.h"
-
-#include <kcharsets.h>
-#include <kcompletionbox.h>
-#include <kcursor.h>
-#include <kcombobox.h>
-#include <kstdaccel.h>
-#include <kpopupmenu.h>
-#include <kedittoolbar.h>
-#include <kkeydialog.h>
-#include <kdebug.h>
-#include <kfiledialog.h>
-#include <kwin.h>
-#include <kinputdialog.h>
-#include <kmessagebox.h>
-#include <kurldrag.h>
-#include <kio/scheduler.h>
-#include <ktempfile.h>
-#include <klocale.h>
-#include <kapplication.h>
-#include <kstatusbar.h>
-#include <kaction.h>
-#include <kstdaction.h>
-#include <kdirwatch.h>
-#include <kstdguiitem.h>
-#include <kiconloader.h>
-#include <kpushbutton.h>
-#include <kuserprofile.h>
-#include <krun.h>
-#include <ktempdir.h>
-//#include <keditlistbox.h>
-#include "globalsettings.h"
-#include "replyphrases.h"
-
-#include <kspell.h>
-#include <kspelldlg.h>
-#include <spellingfilter.h>
-#include <ksyntaxhighlighter.h>
-#include <kcolordialog.h>
-#include <kzip.h>
-#include <ksavefile.h>
-
-#include <qtabdialog.h>
-#include <qregexp.h>
-#include <qbuffer.h>
-#include <qtooltip.h>
-#include <qtextcodec.h>
+#include <qcheckbox.h>
 #include <qheader.h>
-#include <qwhatsthis.h>
-#include <qfontdatabase.h>
 
-#include <mimelib/mimepp.h>
-
-#include <algorithm>
-
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <assert.h>
-
-KMAtmListViewItem::KMAtmListViewItem(QListView *parent)
+KMAtmListViewItem::KMAtmListViewItem( QListView *parent )
   : QObject(),
-    QListViewItem( parent ),
-    mListview( parent ),
-    mCBSignEnabled( false ),
-    mCBEncryptEnabled( false )
+    QListViewItem( parent )
 {
-  mCBEncrypt = new QCheckBox( mListview->viewport() );
-  mCBSign = new QCheckBox( mListview->viewport() );
-  mCBCompress = new QCheckBox( mListview->viewport() );
-  connect( mCBCompress, SIGNAL( clicked() ), this, SLOT( slotCompress() ) );
+  mCBCompress = new QCheckBox( listView()->viewport() );
+  mCBEncrypt = new QCheckBox( listView()->viewport() );
+  mCBSign = new QCheckBox( listView()->viewport() );
+  mCBCompress->setShown( true );
+  updateAllCheckBoxes();
 
-  mCBEncrypt->hide();
-  mCBSign->hide();
+  connect( mCBCompress, SIGNAL( clicked() ), this, SLOT( slotCompress() ) );
+  connect( listView()->header(), SIGNAL( sizeChange(int, int, int) ),
+           SLOT( slotHeaderChange( int, int, int ) ) );
+  connect( listView()->header(), SIGNAL( indexChange(int, int, int) ),
+           SLOT( slotHeaderChange( int, int, int ) ) );
+  connect( listView()->header(), SIGNAL( clicked( int ) ), SLOT( slotHeaderClick( int ) ) );
 }
 
 KMAtmListViewItem::~KMAtmListViewItem()
@@ -154,118 +37,110 @@
   mCBCompress = 0;
 }
 
-void KMAtmListViewItem::paintCell( QPainter * p, const QColorGroup & cg,
-                                  int column, int width, int align )
+void KMAtmListViewItem::updateCheckBox( int headerSection, QCheckBox *cb )
 {
-  // this is also called for the encrypt/sign columns to assure that the
-  // background is cleared
-  QListViewItem::paintCell( p, cg, column, width, align );
-  if ( 4 == column ) {
-    QRect r = mListview->itemRect( this );
-    if ( !r.size().isValid() ) {
-        mListview->ensureItemVisible( this );
-        mListview->repaintContents( FALSE );
-        r = mListview->itemRect( this );
-    }
-    int colWidth = mListview->header()->sectionSize( column );
-    r.setX( mListview->header()->sectionPos( column )
-            - mListview->header()->offset()
-            + colWidth / 2
-            - r.height() / 2
-            - 1 );
-    r.setY( r.y() + 1 );
-    r.setWidth(  r.height() - 2 );
-    r.setHeight( r.height() - 2 );
-    r = QRect( mListview->viewportToContents( r.topLeft() ), r.size() );
+  //Calculate some values to determine the x-position where the checkbox
+  //will be drawn
+  int sectionWidth = listView()->header()->sectionSize( headerSection );
+  int sectionPos = listView()->header()->sectionPos( headerSection );
+  int sectionOffset = sectionWidth / 2 - height() / 4;
 
-    mCBCompress->resize( r.size() );
-    mListview->moveChild( mCBCompress, r.x(), r.y() );
+  //Resize and move the checkbox
+  cb->resize( sectionWidth - sectionOffset - 1, height() - 2 );
+  listView()->moveChild( cb, sectionPos + sectionOffset, itemPos() + 1 );
 
-    QColor bg;
-    if (isSelected())
-      bg = cg.highlight();
-    else
-      bg = cg.base();
-
-    mCBCompress->setPaletteBackgroundColor(bg);
-    mCBCompress->show();
+  //Set the correct background color
+  QColor bg;
+  if ( isSelected() ) {
+    bg = listView()->colorGroup().highlight();
+  } else {
+    bg = listView()->colorGroup().base();
   }
-  if( 5 == column || 6 == column ) {
-    QRect r = mListview->itemRect( this );
-    if ( !r.size().isValid() ) {
-        mListview->ensureItemVisible( this );
-        mListview->repaintContents( FALSE );
-        r = mListview->itemRect( this );
-    }
-    int colWidth = mListview->header()->sectionSize( column );
-    r.setX( mListview->header()->sectionPos( column )
-            + colWidth / 2
-            - r.height() / 2
-            - 1 );
-    r.setY( r.y() + 1 );
-    r.setWidth(  r.height() - 2 );
-    r.setHeight( r.height() - 2 );
-    r = QRect( mListview->viewportToContents( r.topLeft() ), r.size() );
+  cb->setPaletteBackgroundColor( bg );
+}
 
-    QCheckBox* cb = (5 == column) ? mCBEncrypt : mCBSign;
-    cb->resize( r.size() );
-    mListview->moveChild( cb, r.x(), r.y() );
+void KMAtmListViewItem::updateAllCheckBoxes()
+{
+  updateCheckBox( 4, mCBCompress );
+  updateCheckBox( 5, mCBEncrypt );
+  updateCheckBox( 6, mCBSign );
+}
 
-    QColor bg;
-    if (isSelected())
-      bg = cg.highlight();
-    else
-      bg = cg.base();
-
-    bool enabled = (5 == column) ? mCBEncryptEnabled : mCBSignEnabled;
-    cb->setPaletteBackgroundColor(bg);
-    if (enabled) cb->show();
+// Each time a cell is about to be painted, the item's checkboxes are updated
+// as well. This is necessary to keep the positions of the checkboxes
+// up-to-date. The signals which are, in the constructor of this class,
+// connected to the update slots are not sufficent because unfortunatly,
+// Qt does not provide a signal for changed item positions, e.g. during
+// deleting or adding items. The problem with this is that this function does
+// not catch updates which are off-screen, which means under some circumstances
+// checkboxes have invalid positions. This should not happen anymore, but was
+// the cause of bug 113458. Therefore, both the signals connected in the
+// constructor and this function are necessary to keep the checkboxes'
+// positions in sync, and hopefully is enough.
+void KMAtmListViewItem::paintCell ( QPainter * p, const QColorGroup &cg,
+                                    int column, int width, int align )
+{
+  switch ( column ) {
+    case 4: updateCheckBox( 4, mCBCompress ); break;
+    case 5: updateCheckBox( 5, mCBEncrypt ); break;
+    case 6: updateCheckBox( 6, mCBSign ); break;
   }
+
+  QListViewItem::paintCell( p, cg, column, width, align );
 }
 
-void KMAtmListViewItem::enableCryptoCBs(bool on)
+int KMAtmListViewItem::compare( QListViewItem *i, int col, bool ascending ) const
 {
-  if( mCBEncrypt ) {
-    mCBEncryptEnabled = on;
-    mCBEncrypt->setEnabled( on );
-    mCBEncrypt->setShown( on );
+  if ( col != 1 ) {
+    return QListViewItem::compare( i, col, ascending );
   }
-  if( mCBSign ) {
-    mCBSignEnabled = on;
-    mCBSign->setEnabled( on );
-    mCBSign->setShown( on );
-  }
+
+  return mAttachmentSize -
+    (static_cast<KMAtmListViewItem*>(i))->mAttachmentSize;
 }
 
-void KMAtmListViewItem::setEncrypt(bool on)
+void KMAtmListViewItem::enableCryptoCBs( bool on )
 {
-  if( mCBEncrypt )
+  // Show/Hide the appropriate checkboxes.
+  // This should not be necessary because the caller hides the columns
+  // containing the checkboxes anyway.
+  mCBEncrypt->setShown( on );
+  mCBSign->setShown( on );
+}
+
+void KMAtmListViewItem::setEncrypt( bool on )
+{
+  if ( mCBEncrypt ) {
     mCBEncrypt->setChecked( on );
+  }
 }
 
 bool KMAtmListViewItem::isEncrypt()
 {
-  if( mCBEncrypt )
+  if ( mCBEncrypt ) {
     return mCBEncrypt->isChecked();
-  else
+  } else {
     return false;
+  }
 }
 
-void KMAtmListViewItem::setSign(bool on)
+void KMAtmListViewItem::setSign( bool on )
 {
-  if( mCBSign )
+  if ( mCBSign ) {
     mCBSign->setChecked( on );
+  }
 }
 
 bool KMAtmListViewItem::isSign()
 {
-  if( mCBSign )
+  if ( mCBSign ) {
     return mCBSign->isChecked();
-  else
+  } else {
     return false;
+  }
 }
 
-void KMAtmListViewItem::setCompress(bool on)
+void KMAtmListViewItem::setCompress( bool on )
 {
   mCBCompress->setChecked( on );
 }
@@ -277,10 +152,24 @@
 
 void KMAtmListViewItem::slotCompress()
 {
-    if ( mCBCompress->isChecked() )
-        emit compress( itemPos() );
-    else
-        emit uncompress( itemPos() );
+  if ( mCBCompress->isChecked() ) {
+    emit compress( itemPos() );
+  } else {
+    emit uncompress( itemPos() );
+  }
 }
 
+// Update the item's checkboxes when the position of those change
+// due to different column positions
+void KMAtmListViewItem::slotHeaderChange ( int, int, int )
+{
+  updateAllCheckBoxes();
+}
+
+//Update the item's checkboxes when the list is being sorted
+void KMAtmListViewItem::slotHeaderClick( int )
+{
+  updateAllCheckBoxes();
+}
+
 #include "kmatmlistview.moc"
--- branches/work/kdepim-3.5.5+/kmail/kmatmlistview.h #602742:602743
@@ -1,5 +1,5 @@
 /* -*- mode: C++; c-file-style: "gnu" -*-
- * KMComposeWin Header File
+ * KMAtmListViewItem Header File
  * Author: Markus Wuebben <markus.wuebben@kde.org>
  */
 #ifndef __KMAIL_KMATMLISTVIEW_H__
@@ -15,48 +15,57 @@
 class KMAtmListViewItem : public QObject, public QListViewItem
 {
   Q_OBJECT
-  friend class ::KMComposeWin;
-  friend class ::MessageComposer;
 
 public:
-  KMAtmListViewItem(QListView * parent);
+  KMAtmListViewItem( QListView *parent );
   virtual ~KMAtmListViewItem();
-  virtual void paintCell( QPainter * p, const QColorGroup & cg,
-                          int column, int width, int align );
 
+  //A custom compare function is needed because the size column is
+  //human-readable and therefore doesn't sort correctly.
+  virtual int compare( QListViewItem *i, int col, bool ascending ) const;
+
+  virtual void paintCell ( QPainter * p, const QColorGroup & cg, int column, int width, int align );
+
   void setUncompressedMimeType( const QCString & type, const QCString & subtype ) {
     mType = type; mSubtype = subtype;
   }
+  void setAttachmentSize( int numBytes ) {
+    mAttachmentSize = numBytes;
+  }
   void uncompressedMimeType( QCString & type, QCString & subtype ) const {
     type = mType; subtype = mSubtype;
   }
-  void setUncompressedCodec( const QCString & codec ) { mCodec = codec; }
+  void setUncompressedCodec( const QCString &codec ) { mCodec = codec; }
   QCString uncompressedCodec() const { return mCodec; }
 
+  void enableCryptoCBs( bool on );
+  void setEncrypt( bool on );
+  bool isEncrypt();
+  void setSign( bool on );
+  bool isSign();
+  void setCompress( bool on );
+  bool isCompress();
+
 signals:
   void compress( int );
   void uncompress( int );
 
-protected:
-  void enableCryptoCBs(bool on);
-  void setEncrypt(bool on);
-  bool isEncrypt();
-  void setSign(bool on);
-  bool isSign();
-  void setCompress(bool on);
-  bool isCompress();
-
 private slots:
   void slotCompress();
+  void slotHeaderChange( int, int, int );
+  void slotHeaderClick( int );
 
+protected:
+
+  void updateCheckBox( int headerSection, QCheckBox *cb );
+  void updateAllCheckBoxes();
+
 private:
-  QListView* mListview;
-  QCheckBox* mCBEncrypt;
-  QCheckBox* mCBSign;
-  QCheckBox* mCBCompress;
-  bool mCBSignEnabled, mCBEncryptEnabled;
+  QCheckBox *mCBEncrypt;
+  QCheckBox *mCBSign;
+  QCheckBox *mCBCompress;
   QCString mType, mSubtype, mCodec;
+  int mAttachmentSize;
 };
 
-
 #endif // __KMAIL_KMATMLISTVIEW_H__
--- branches/work/kdepim-3.5.5+/kmail/kmcomposewin.cpp #602742:602743
@@ -2307,6 +2307,7 @@
   lvi->setText(1, KIO::convertSize( msgPart->decodedSize()));
   lvi->setText(2, msgPart->contentTransferEncodingStr());
   lvi->setText(3, prettyMimeType(msgPart->typeStr() + "/" + msgPart->subtypeStr()));
+  lvi->setAttachmentSize(msgPart->decodedSize());
 
   if ( loadDefaults ) {
     if( canSignEncryptAttachments() ) {
Comment 13 Allen Winter 2006-11-06 19:25:41 UTC
SVN commit 602752 by winterz:

merge SVN commit 602743 by winterz:

Patch from Thomas McGuire that fixes the bugs:
+ Wrong checkbox in the layout when adding attachments to the composer
+ attachment display in editor: "sort by size" sorts alphanumerically, not by size

Thanks for the patch Thomas and sorry it took so long.

BUGS: 113458, 119526


 M  +104 -215  kmatmlistview.cpp  
 M  +31 -22    kmatmlistview.h  
 M  +1 -0      kmcomposewin.cpp  


--- branches/KDE/3.5/kdepim/kmail/kmatmlistview.cpp #602751:602752
@@ -1,147 +1,30 @@
 // -*- mode: C++; c-file-style: "gnu" -*-
-// kmcomposewin.cpp
+// kmatmlistview.cpp
 // Author: Markus Wuebben <markus.wuebben@kde.org>
 // This code is published under the GPL.
 
 #include <config.h>
 
 #include "kmatmlistview.h"
-
-#include "kmmainwin.h"
-#include "kmreadermainwin.h"
-#include "messagesender.h"
-#include "kmmsgpartdlg.h"
-#include <kpgpblock.h>
-#include <kaddrbook.h>
-#include "kmaddrbook.h"
-#include "kmmsgdict.h"
-#include "kmfolderimap.h"
-#include "kmfoldermgr.h"
-#include "kmfoldercombobox.h"
-#include "kmtransport.h"
-#include "kmcommands.h"
-#include "kcursorsaver.h"
-#include "partNode.h"
-#include "attachmentlistview.h"
-#include "transportmanager.h"
-using KMail::AttachmentListView;
-#include "dictionarycombobox.h"
-using KMail::DictionaryComboBox;
-#include "addressesdialog.h"
-using KPIM::AddressesDialog;
-#include "addresseeemailselection.h"
-using KPIM::AddresseeEmailSelection;
-using KPIM::AddresseeSelectorDialog;
-#include <maillistdrag.h>
-using KPIM::MailListDrag;
-#include "recentaddresses.h"
-using KRecentAddress::RecentAddresses;
-#include "kleo_util.h"
-#include "stl_util.h"
-#include "recipientseditor.h"
-
-#include "attachmentcollector.h"
-#include "objecttreeparser.h"
-
-#include "kmfoldermaildir.h"
-
-#include <libkpimidentities/identitymanager.h>
-#include <libkpimidentities/identitycombo.h>
-#include <libkpimidentities/identity.h>
-#include <libkdepim/kfileio.h>
-#include <libemailfunctions/email.h>
-#include <kleo/cryptobackendfactory.h>
-#include <kleo/exportjob.h>
-#include <ui/progressdialog.h>
-#include <ui/keyselectiondialog.h>
-
-#include <gpgmepp/context.h>
-#include <gpgmepp/key.h>
-
-#include <kabc/vcardconverter.h>
-#include <libkdepim/kvcarddrag.h>
-#include <kio/netaccess.h>
-
-
-#include "klistboxdialog.h"
-
-#include "messagecomposer.h"
-
-#include <kcharsets.h>
-#include <kcompletionbox.h>
-#include <kcursor.h>
-#include <kcombobox.h>
-#include <kstdaccel.h>
-#include <kpopupmenu.h>
-#include <kedittoolbar.h>
-#include <kkeydialog.h>
-#include <kdebug.h>
-#include <kfiledialog.h>
-#include <kwin.h>
-#include <kinputdialog.h>
-#include <kmessagebox.h>
-#include <kurldrag.h>
-#include <kio/scheduler.h>
-#include <ktempfile.h>
-#include <klocale.h>
-#include <kapplication.h>
-#include <kstatusbar.h>
-#include <kaction.h>
-#include <kstdaction.h>
-#include <kdirwatch.h>
-#include <kstdguiitem.h>
-#include <kiconloader.h>
-#include <kpushbutton.h>
-#include <kuserprofile.h>
-#include <krun.h>
-#include <ktempdir.h>
-//#include <keditlistbox.h>
-#include "globalsettings.h"
-#include "replyphrases.h"
-
-#include <kspell.h>
-#include <kspelldlg.h>
-#include <spellingfilter.h>
-#include <ksyntaxhighlighter.h>
-#include <kcolordialog.h>
-#include <kzip.h>
-#include <ksavefile.h>
-
-#include <qtabdialog.h>
-#include <qregexp.h>
-#include <qbuffer.h>
-#include <qtooltip.h>
-#include <qtextcodec.h>
+#include <qcheckbox.h>
 #include <qheader.h>
-#include <qwhatsthis.h>
-#include <qfontdatabase.h>
 
-#include <mimelib/mimepp.h>
-
-#include <algorithm>
-
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <errno.h>
-#include <fcntl.h>
-#include <assert.h>
-
-KMAtmListViewItem::KMAtmListViewItem(QListView *parent)
+KMAtmListViewItem::KMAtmListViewItem( QListView *parent )
   : QObject(),
-    QListViewItem( parent ),
-    mListview( parent ),
-    mCBSignEnabled( false ),
-    mCBEncryptEnabled( false )
+    QListViewItem( parent )
 {
-  mCBEncrypt = new QCheckBox( mListview->viewport() );
-  mCBSign = new QCheckBox( mListview->viewport() );
-  mCBCompress = new QCheckBox( mListview->viewport() );
-  connect( mCBCompress, SIGNAL( clicked() ), this, SLOT( slotCompress() ) );
+  mCBCompress = new QCheckBox( listView()->viewport() );
+  mCBEncrypt = new QCheckBox( listView()->viewport() );
+  mCBSign = new QCheckBox( listView()->viewport() );
+  mCBCompress->setShown( true );
+  updateAllCheckBoxes();
 
-  mCBEncrypt->hide();
-  mCBSign->hide();
+  connect( mCBCompress, SIGNAL( clicked() ), this, SLOT( slotCompress() ) );
+  connect( listView()->header(), SIGNAL( sizeChange(int, int, int) ),
+           SLOT( slotHeaderChange( int, int, int ) ) );
+  connect( listView()->header(), SIGNAL( indexChange(int, int, int) ),
+           SLOT( slotHeaderChange( int, int, int ) ) );
+  connect( listView()->header(), SIGNAL( clicked( int ) ), SLOT( slotHeaderClick( int ) ) );
 }
 
 KMAtmListViewItem::~KMAtmListViewItem()
@@ -154,118 +37,110 @@
   mCBCompress = 0;
 }
 
-void KMAtmListViewItem::paintCell( QPainter * p, const QColorGroup & cg,
-                                  int column, int width, int align )
+void KMAtmListViewItem::updateCheckBox( int headerSection, QCheckBox *cb )
 {
-  // this is also called for the encrypt/sign columns to assure that the
-  // background is cleared
-  QListViewItem::paintCell( p, cg, column, width, align );
-  if ( 4 == column ) {
-    QRect r = mListview->itemRect( this );
-    if ( !r.size().isValid() ) {
-        mListview->ensureItemVisible( this );
-        mListview->repaintContents( FALSE );
-        r = mListview->itemRect( this );
-    }
-    int colWidth = mListview->header()->sectionSize( column );
-    r.setX( mListview->header()->sectionPos( column )
-            - mListview->header()->offset()
-            + colWidth / 2
-            - r.height() / 2
-            - 1 );
-    r.setY( r.y() + 1 );
-    r.setWidth(  r.height() - 2 );
-    r.setHeight( r.height() - 2 );
-    r = QRect( mListview->viewportToContents( r.topLeft() ), r.size() );
+  //Calculate some values to determine the x-position where the checkbox
+  //will be drawn
+  int sectionWidth = listView()->header()->sectionSize( headerSection );
+  int sectionPos = listView()->header()->sectionPos( headerSection );
+  int sectionOffset = sectionWidth / 2 - height() / 4;
 
-    mCBCompress->resize( r.size() );
-    mListview->moveChild( mCBCompress, r.x(), r.y() );
+  //Resize and move the checkbox
+  cb->resize( sectionWidth - sectionOffset - 1, height() - 2 );
+  listView()->moveChild( cb, sectionPos + sectionOffset, itemPos() + 1 );
 
-    QColor bg;
-    if (isSelected())
-      bg = cg.highlight();
-    else
-      bg = cg.base();
-
-    mCBCompress->setPaletteBackgroundColor(bg);
-    mCBCompress->show();
+  //Set the correct background color
+  QColor bg;
+  if ( isSelected() ) {
+    bg = listView()->colorGroup().highlight();
+  } else {
+    bg = listView()->colorGroup().base();
   }
-  if( 5 == column || 6 == column ) {
-    QRect r = mListview->itemRect( this );
-    if ( !r.size().isValid() ) {
-        mListview->ensureItemVisible( this );
-        mListview->repaintContents( FALSE );
-        r = mListview->itemRect( this );
-    }
-    int colWidth = mListview->header()->sectionSize( column );
-    r.setX( mListview->header()->sectionPos( column )
-            + colWidth / 2
-            - r.height() / 2
-            - 1 );
-    r.setY( r.y() + 1 );
-    r.setWidth(  r.height() - 2 );
-    r.setHeight( r.height() - 2 );
-    r = QRect( mListview->viewportToContents( r.topLeft() ), r.size() );
+  cb->setPaletteBackgroundColor( bg );
+}
 
-    QCheckBox* cb = (5 == column) ? mCBEncrypt : mCBSign;
-    cb->resize( r.size() );
-    mListview->moveChild( cb, r.x(), r.y() );
+void KMAtmListViewItem::updateAllCheckBoxes()
+{
+  updateCheckBox( 4, mCBCompress );
+  updateCheckBox( 5, mCBEncrypt );
+  updateCheckBox( 6, mCBSign );
+}
 
-    QColor bg;
-    if (isSelected())
-      bg = cg.highlight();
-    else
-      bg = cg.base();
-
-    bool enabled = (5 == column) ? mCBEncryptEnabled : mCBSignEnabled;
-    cb->setPaletteBackgroundColor(bg);
-    if (enabled) cb->show();
+// Each time a cell is about to be painted, the item's checkboxes are updated
+// as well. This is necessary to keep the positions of the checkboxes
+// up-to-date. The signals which are, in the constructor of this class,
+// connected to the update slots are not sufficent because unfortunatly,
+// Qt does not provide a signal for changed item positions, e.g. during
+// deleting or adding items. The problem with this is that this function does
+// not catch updates which are off-screen, which means under some circumstances
+// checkboxes have invalid positions. This should not happen anymore, but was
+// the cause of bug 113458. Therefore, both the signals connected in the
+// constructor and this function are necessary to keep the checkboxes'
+// positions in sync, and hopefully is enough.
+void KMAtmListViewItem::paintCell ( QPainter * p, const QColorGroup &cg,
+                                    int column, int width, int align )
+{
+  switch ( column ) {
+    case 4: updateCheckBox( 4, mCBCompress ); break;
+    case 5: updateCheckBox( 5, mCBEncrypt ); break;
+    case 6: updateCheckBox( 6, mCBSign ); break;
   }
+
+  QListViewItem::paintCell( p, cg, column, width, align );
 }
 
-void KMAtmListViewItem::enableCryptoCBs(bool on)
+int KMAtmListViewItem::compare( QListViewItem *i, int col, bool ascending ) const
 {
-  if( mCBEncrypt ) {
-    mCBEncryptEnabled = on;
-    mCBEncrypt->setEnabled( on );
-    mCBEncrypt->setShown( on );
+  if ( col != 1 ) {
+    return QListViewItem::compare( i, col, ascending );
   }
-  if( mCBSign ) {
-    mCBSignEnabled = on;
-    mCBSign->setEnabled( on );
-    mCBSign->setShown( on );
-  }
+
+  return mAttachmentSize -
+    (static_cast<KMAtmListViewItem*>(i))->mAttachmentSize;
 }
 
-void KMAtmListViewItem::setEncrypt(bool on)
+void KMAtmListViewItem::enableCryptoCBs( bool on )
 {
-  if( mCBEncrypt )
+  // Show/Hide the appropriate checkboxes.
+  // This should not be necessary because the caller hides the columns
+  // containing the checkboxes anyway.
+  mCBEncrypt->setShown( on );
+  mCBSign->setShown( on );
+}
+
+void KMAtmListViewItem::setEncrypt( bool on )
+{
+  if ( mCBEncrypt ) {
     mCBEncrypt->setChecked( on );
+  }
 }
 
 bool KMAtmListViewItem::isEncrypt()
 {
-  if( mCBEncrypt )
+  if ( mCBEncrypt ) {
     return mCBEncrypt->isChecked();
-  else
+  } else {
     return false;
+  }
 }
 
-void KMAtmListViewItem::setSign(bool on)
+void KMAtmListViewItem::setSign( bool on )
 {
-  if( mCBSign )
+  if ( mCBSign ) {
     mCBSign->setChecked( on );
+  }
 }
 
 bool KMAtmListViewItem::isSign()
 {
-  if( mCBSign )
+  if ( mCBSign ) {
     return mCBSign->isChecked();
-  else
+  } else {
     return false;
+  }
 }
 
-void KMAtmListViewItem::setCompress(bool on)
+void KMAtmListViewItem::setCompress( bool on )
 {
   mCBCompress->setChecked( on );
 }
@@ -277,10 +152,24 @@
 
 void KMAtmListViewItem::slotCompress()
 {
-    if ( mCBCompress->isChecked() )
-        emit compress( itemPos() );
-    else
-        emit uncompress( itemPos() );
+  if ( mCBCompress->isChecked() ) {
+    emit compress( itemPos() );
+  } else {
+    emit uncompress( itemPos() );
+  }
 }
 
+// Update the item's checkboxes when the position of those change
+// due to different column positions
+void KMAtmListViewItem::slotHeaderChange ( int, int, int )
+{
+  updateAllCheckBoxes();
+}
+
+//Update the item's checkboxes when the list is being sorted
+void KMAtmListViewItem::slotHeaderClick( int )
+{
+  updateAllCheckBoxes();
+}
+
 #include "kmatmlistview.moc"
--- branches/KDE/3.5/kdepim/kmail/kmatmlistview.h #602751:602752
@@ -1,5 +1,5 @@
 /* -*- mode: C++; c-file-style: "gnu" -*-
- * KMComposeWin Header File
+ * KMAtmListViewItem Header File
  * Author: Markus Wuebben <markus.wuebben@kde.org>
  */
 #ifndef __KMAIL_KMATMLISTVIEW_H__
@@ -15,48 +15,57 @@
 class KMAtmListViewItem : public QObject, public QListViewItem
 {
   Q_OBJECT
-  friend class ::KMComposeWin;
-  friend class ::MessageComposer;
 
 public:
-  KMAtmListViewItem(QListView * parent);
+  KMAtmListViewItem( QListView *parent );
   virtual ~KMAtmListViewItem();
-  virtual void paintCell( QPainter * p, const QColorGroup & cg,
-                          int column, int width, int align );
 
+  //A custom compare function is needed because the size column is
+  //human-readable and therefore doesn't sort correctly.
+  virtual int compare( QListViewItem *i, int col, bool ascending ) const;
+
+  virtual void paintCell ( QPainter * p, const QColorGroup & cg, int column, int width, int align );
+
   void setUncompressedMimeType( const QCString & type, const QCString & subtype ) {
     mType = type; mSubtype = subtype;
   }
+  void setAttachmentSize( int numBytes ) {
+    mAttachmentSize = numBytes;
+  }
   void uncompressedMimeType( QCString & type, QCString & subtype ) const {
     type = mType; subtype = mSubtype;
   }
-  void setUncompressedCodec( const QCString & codec ) { mCodec = codec; }
+  void setUncompressedCodec( const QCString &codec ) { mCodec = codec; }
   QCString uncompressedCodec() const { return mCodec; }
 
+  void enableCryptoCBs( bool on );
+  void setEncrypt( bool on );
+  bool isEncrypt();
+  void setSign( bool on );
+  bool isSign();
+  void setCompress( bool on );
+  bool isCompress();
+
 signals:
   void compress( int );
   void uncompress( int );
 
-protected:
-  void enableCryptoCBs(bool on);
-  void setEncrypt(bool on);
-  bool isEncrypt();
-  void setSign(bool on);
-  bool isSign();
-  void setCompress(bool on);
-  bool isCompress();
-
 private slots:
   void slotCompress();
+  void slotHeaderChange( int, int, int );
+  void slotHeaderClick( int );
 
+protected:
+
+  void updateCheckBox( int headerSection, QCheckBox *cb );
+  void updateAllCheckBoxes();
+
 private:
-  QListView* mListview;
-  QCheckBox* mCBEncrypt;
-  QCheckBox* mCBSign;
-  QCheckBox* mCBCompress;
-  bool mCBSignEnabled, mCBEncryptEnabled;
+  QCheckBox *mCBEncrypt;
+  QCheckBox *mCBSign;
+  QCheckBox *mCBCompress;
   QCString mType, mSubtype, mCodec;
+  int mAttachmentSize;
 };
 
-
 #endif // __KMAIL_KMATMLISTVIEW_H__
--- branches/KDE/3.5/kdepim/kmail/kmcomposewin.cpp #602751:602752
@@ -2273,6 +2273,7 @@
   lvi->setText(1, KIO::convertSize( msgPart->decodedSize()));
   lvi->setText(2, msgPart->contentTransferEncodingStr());
   lvi->setText(3, prettyMimeType(msgPart->typeStr() + "/" + msgPart->subtypeStr()));
+  lvi->setAttachmentSize(msgPart->decodedSize());
 
   if ( loadDefaults ) {
     if( canSignEncryptAttachments() ) {
Comment 14 Thomas McGuire 2007-01-28 17:38:34 UTC
*** Bug 125742 has been marked as a duplicate of this bug. ***