Bug 149706

Summary: Wrong or bizarre behaviour in the thumbnails selection
Product: [Applications] kphotoalbum Reporter: Baptiste MATHUS <ml>
Component: Thumbnail ViewerAssignee: KPhotoAlbum Bugs <kpabugs>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: SVN (KDE3 branch)   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Baptiste MATHUS 2007-09-09 16:35:24 UTC 
Version:            (using KDE KDE 3.5.6)
Installed from:    Compiled From Sources
OS:                Linux

If I had to explain how KPA works, I wouldn't be able to do it...

This bug only occurs when using the mouse, then the keyboard to make a selection. Click somewhere, then press shift+some arrow to see.

To workaround this bug, I've now been used to click, then press right arrow, left arrow to "put" the right place for KPA. Then, and only then I shift/select my thumbs.

I think there's something wrong the index management when selecting with mouse or keyboard.

Example: 
* Open the KPA with demo db
* Show thumbnails, my thumbs size lets me show 5 images
* Click on say, Donna (the black dog on the second line, last image with my thumb size)
* Shift+up arrow => select Donna and Spiff (the rabbit), but nothing else ?!?
* Shift+left arrow => Donna is not selected anymore, now both thumbs of Spiff (top right corner) are selected

=> From how KPA's selection model works, clicking on Donna, then shift-up arrow, should have selected the whole second line + Spiff. Isn't it?

Another astonishing one:
* Use the keyboard to select only the first thumbnail (Jesper, The Cure's mode)
* Click once on Jesper in front of the grand canyon (second line, middle image)
* Shift-down => Everything is selected until Jesper in snooker. Even first line and images before the one clicked. In this case, I just think the selected index is not always correctly set using the mouse, although I NEVER had any problem if I force myself to use only the keyboard.

Right, HTH and that this bug is reproducible. Let me know if you need more informations.
Cheers
Comment 1 Tuomas Suutari 2007-09-11 00:02:11 UTC 
Yep, this really is bizarre behaviour.
Comment 2 Tuomas Suutari 2007-09-11 11:29:47 UTC 
SVN commit 711055 by suutari:

Bugfix: Fix behaviour of thumbnail selecting with keyboard.

BUG: 149706


 M  +4 -0      ChangeLog  
 M  +16 -9     ThumbnailView/ThumbnailWidget.cpp  
 M  +1 -1      ThumbnailView/ThumbnailWidget.h  


--- branches/extragear/kde3/graphics/kphotoalbum/ChangeLog #711054:711055
@@ -1,3 +1,7 @@
+2007-09-11  Tuomas Suutari  <thsuut@utu.fi>
+
+	* Fix behaviour of thumbnail selecting with keyboard. Fixes bug 149706.
+
 2007-08-18  Jan Kundrat  <jkt@gentoo.org>
 
 	* Use libkdcraw intead of budled and rotten dcraw copy
--- branches/extragear/kde3/graphics/kphotoalbum/ThumbnailView/ThumbnailWidget.cpp #711054:711055
@@ -521,8 +521,6 @@
 
 void ThumbnailView::ThumbnailWidget::keyboardMoveEvent( QKeyEvent* event )
 {
-    static QString startPossition;
-
     if ( !( event->state()& ShiftButton ) && !( event->state() &  ControlButton ) ) {
         clearSelection();
     }
@@ -534,6 +532,13 @@
 
     // Update current position if it is outside view and we do not have any modifiers
     // that is if we just scroll arround.
+    //
+    // Use case is following: There is a selected item which is not
+    // visible because user has scrolled by other means than the
+    // keyboard (scrollbar or mouse wheel). In that case if the user
+    // presses keyboard movement key, the selection is forgotten and
+    // instead a currently visible cell is selected. So no scrolling
+    // of the view will be done.
     if ( !( event->state()& ShiftButton ) && !( event->state() &  ControlButton ) ) {
         if ( currentPos.row() < firstVisibleRow( PartlyVisible ) )
             currentPos = Cell( firstVisibleRow( FullyVisible ), currentPos.col() );
@@ -593,9 +598,13 @@
     if ( newPos < Cell(0,0) )
         newPos = Cell(0,0);
 
+    // Without modifiers, clear the old selection
+    if ( !(event->state() & ShiftButton) && !(event->state() & ControlButton) )
+        _selectedFiles.clear();
+
     // Update focus cell, and set selection
-    if ( (event->state() & ShiftButton) && !startPossition.isEmpty() )
-        selectItems( positionForFileName( startPossition ), newPos );
+    if ( (event->state() & ShiftButton) )
+        selectItems( currentPos, newPos, false );
 
     if ( ! (event->state() & ControlButton ) ) {
         selectCell( newPos );
@@ -603,9 +612,6 @@
     }
     _currentItem = fileNameInCell( newPos );
 
-    if ( !( event->state() & ShiftButton ) || startPossition.isEmpty() )
-        startPossition = _currentItem;
-
     // Scroll if necesary
     if ( newPos.row() > lastVisibleRow( ThumbnailWidget::FullyVisible ) )
         setContentsPos( contentsX(), cellGeometry( newPos.row(), newPos.col() ).top() -
@@ -618,10 +624,11 @@
 /**
  * Update selection to include files from start to end
  */
-void ThumbnailView::ThumbnailWidget::selectItems( const Cell& start, const Cell& end )
+void ThumbnailView::ThumbnailWidget::selectItems( const Cell& start, const Cell& end, bool doClear )
 {
     Set<QString> oldSelection = _selectedFiles;
-    _selectedFiles.clear();
+    if (doClear)
+        _selectedFiles.clear();
 
     selectAllCellsBetween( start, end, false );
 
--- branches/extragear/kde3/graphics/kphotoalbum/ThumbnailView/ThumbnailWidget.h #711054:711055
@@ -133,7 +133,7 @@
     // Misc
     void updateGridSize();
     bool isMovementKey( int key );
-    void selectItems( const Cell& start, const Cell& end );
+    void selectItems( const Cell& start, const Cell& end, bool doClear = true );
     void ensureCellsSorted( Cell& pos1, Cell& pos2 );
     QStringList reverseList( const QStringList& ) const;
     QValueVector<QString> reverseVector( const QValueVector<QString>& ) const;