Bug 119526

Summary: JJ: attachment display in editor: "sort by size" sorts alphanumerically, not by size
Product: [Unmaintained] kmail Reporter: Jens <jens-bugs.kde.org>
Component: generalAssignee: kdepim bugs <kdepim-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: mcguire
Priority: NOR    
Version: 1.9.1   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:
Attachments: Patch which fixes the problem

Description Jens 2006-01-04 21:06:07 UTC
Version:           1.9.1 (using KDE 3.5.0 Level "a" , SUSE 10.0 UNSUPPORTED)
Compiler:          Target: i586-suse-linux
OS:                Linux (i686) release 2.6.13-15.7-default

I suspect this is a JJ (please change if not).

Reproduce:
- Create new mail in KMail
- Attach a few files

Problem:
KMail sorts alphanumerically, ie. 42.5kb < 4.3kb < 800 bytes.

Solution: Either sort according to "real" (hidden) value or display true numerical value. This problem was also in the date column in Bug #96994 (Search dialog, sort by date).

Thanks :)

Jens
Comment 1 Thomas McGuire 2006-01-13 23:15:18 UTC
Created attachment 14242 [details]
Patch which fixes the problem

This patch fixes the problem by sorting with the "hidden" value.
The patch is against a recent version of the 3.5 SVN branch, please apply to
the 3.5 branch and trunk.
Comment 2 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 3 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() ) {