Bug 113458

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.

After this, opening an email with attachment I got the same problem.

Additional comment, this happened dragging several files at the same time.

This problem continues happening in kde-3.5_beta2

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.

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).

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.

Why is the patch that big? Is it still valid for the current KMail?

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

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?

Created attachment 18430 [details]
Trunk version of the patch

This patch is against the trunk version of KMail.

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() ) {

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() ) {

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