Bug 149706 - Wrong or bizarre behaviour in the thumbnails selection
Summary: Wrong or bizarre behaviour in the thumbnails selection
Status: RESOLVED FIXED
Alias: None
Product: kphotoalbum
Classification: Applications
Component: Thumbnail Viewer (show other bugs)
Version: SVN (KDE3 branch)
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: KPhotoAlbum Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-09-09 16:35 UTC by Baptiste MATHUS
Modified: 2007-09-11 11:29 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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;