Fix possible endless recursion when using the rubberband

If the autoscrolling has been activated when using the rubberband,
it was possible that an endless recursion occured as the
autoscrolling triggered a change of the rubberband which triggered
a change of the autoscrolling etc.

6 files changed, 79 insertions, 52 deletions

diff --git a/src/kitemviews/kitemlistcontainer.cpp b/src/kitemviews/kitemlistcontainer.cpp
index b01751e46..a0e8c1515 100644
--- a/src/kitemviews/kitemlistcontainer.cpp
+++ b/src/kitemviews/kitemlistcontainer.cpp
@@ -59,8 +59,8 @@ public:
 KItemListContainer::KItemListContainer(KItemListController* controller, QWidget* parent) :
     QAbstractScrollArea(parent),
     m_controller(controller),
-    m_sliderMovedByUser(false),
-    m_viewOffsetAnimation(0)
+    m_smoothScrolling(false),
+    m_smoothScrollingAnimation(0)
 {
     Q_ASSERT(controller);
     controller->setParent(this);
@@ -69,7 +69,9 @@ KItemListContainer::KItemListContainer(KItemListController* controller, QWidget*
 
 KItemListContainer::KItemListContainer(QWidget* parent) :
     QAbstractScrollArea(parent),
-    m_controller(0)
+    m_controller(0),
+    m_smoothScrolling(false),
+    m_smoothScrollingAnimation(0)
 {
     initialize();
 }
@@ -119,24 +121,24 @@ void KItemListContainer::scrollContentsBy(int dx, int dy)
     const qreal currentOffset = view->offset();
     qreal offsetDiff = (view->scrollOrientation() == Qt::Vertical) ? dy : dx;
 
-    const bool animRunning = (m_viewOffsetAnimation->state() == QAbstractAnimation::Running);
+    const bool animRunning = (m_smoothScrollingAnimation->state() == QAbstractAnimation::Running);
     if (animRunning) {
         // Stopping a running animation means skipping the range from the current offset
         // until the target offset. To prevent skipping of the range the difference
         // is added to the new target offset.
-        m_viewOffsetAnimation->stop();
-        const qreal targetOffset = m_viewOffsetAnimation->endValue().toReal();
+        const qreal targetOffset = m_smoothScrollingAnimation->endValue().toReal();
         offsetDiff += (currentOffset - targetOffset);
     }
 
     const qreal newOffset = currentOffset - offsetDiff;
 
-    if (m_sliderMovedByUser || animRunning) {
-        m_viewOffsetAnimation->stop();
-        m_viewOffsetAnimation->setStartValue(currentOffset);
-        m_viewOffsetAnimation->setEndValue(newOffset);
-        m_viewOffsetAnimation->setEasingCurve(animRunning ? QEasingCurve::OutQuad : QEasingCurve::InOutQuad);
-        m_viewOffsetAnimation->start();
+    if (m_smoothScrolling || animRunning) {
+        m_smoothScrollingAnimation->stop();
+        m_smoothScrollingAnimation->setStartValue(currentOffset);
+        m_smoothScrollingAnimation->setEndValue(newOffset);
+        m_smoothScrollingAnimation->setEasingCurve(animRunning ? QEasingCurve::OutQuad : QEasingCurve::InOutQuad);
+        m_smoothScrollingAnimation->start();
+        view->setOffset(currentOffset);
     } else {
         view->setOffset(newOffset);
     }
@@ -147,8 +149,8 @@ bool KItemListContainer::eventFilter(QObject* obj, QEvent* event)
     Q_ASSERT(obj == horizontalScrollBar() || obj == verticalScrollBar());
 
     // Check whether the scrollbar has been adjusted by a mouse-event
-    // triggered by the user and remember this in m_sliderMovedByUser.
-    // The smooth scrolling will only get active if m_sliderMovedByUser
+    // triggered by the user and remember this in m_smoothScrolling.
+    // The smooth scrolling will only get active if m_smoothScrolling
     // is true (see scrollContentsBy()).
     const bool scrollVertical = (m_controller->view()->scrollOrientation() == Qt::Vertical);
     const bool checkEvent = ( scrollVertical && obj == verticalScrollBar()) ||
@@ -156,11 +158,11 @@ bool KItemListContainer::eventFilter(QObject* obj, QEvent* event)
     if (checkEvent) {
         switch (event->type()) {
         case QEvent::MouseButtonPress:
-            m_sliderMovedByUser = true;
-           break;
+            m_smoothScrolling = true;
+            break;
 
         case QEvent::MouseButtonRelease:
-            m_sliderMovedByUser = false;
+            m_smoothScrolling = false;
             break;
 
         case QEvent::Wheel:
@@ -169,7 +171,7 @@ bool KItemListContainer::eventFilter(QObject* obj, QEvent* event)
 
         default:
             break;
-         }
+        }
     }
 
     return QAbstractScrollArea::eventFilter(obj, event);
@@ -185,8 +187,8 @@ void KItemListContainer::wheelEvent(QWheelEvent* event)
     const int numDegrees = event->delta() / 8;
     const int numSteps = numDegrees / 15;
 
-    const bool previous = m_sliderMovedByUser;
-    m_sliderMovedByUser = true;
+    const bool previous = m_smoothScrolling;
+    m_smoothScrolling = true;
     if (view->scrollOrientation() == Qt::Vertical) {
         const int value = verticalScrollBar()->value();
         verticalScrollBar()->setValue(value - numSteps * view->size().height());
@@ -194,7 +196,7 @@ void KItemListContainer::wheelEvent(QWheelEvent* event)
         const int value = horizontalScrollBar()->value();
         horizontalScrollBar()->setValue(value - numSteps * view->size().width());
     }
-    m_sliderMovedByUser = previous;
+    m_smoothScrolling = previous;
 
     event->accept();
 }
@@ -213,17 +215,27 @@ void KItemListContainer::slotViewChanged(KItemListView* current, KItemListView*
         disconnect(previous, SIGNAL(offsetChanged(qreal,qreal)), this, SLOT(updateScrollBars()));
         disconnect(previous, SIGNAL(maximumOffsetChanged(qreal,qreal)), this, SLOT(updateScrollBars()));
         disconnect(previous, SIGNAL(scrollTo(qreal)), this, SLOT(scrollTo(qreal)));
-        m_viewOffsetAnimation->setTargetObject(0);
+        m_smoothScrollingAnimation->setTargetObject(0);
     }
     if (current) {
         scene->addItem(current);
         connect(current, SIGNAL(offsetChanged(qreal,qreal)), this, SLOT(updateScrollBars()));
         connect(current, SIGNAL(maximumOffsetChanged(qreal,qreal)), this, SLOT(updateScrollBars()));
         connect(current, SIGNAL(scrollTo(qreal)), this, SLOT(scrollTo(qreal)));
-        m_viewOffsetAnimation->setTargetObject(current);
+        m_smoothScrollingAnimation->setTargetObject(current);
     }
 }
 
+void KItemListContainer::slotAnimationStateChanged(QAbstractAnimation::State newState,
+                                                   QAbstractAnimation::State oldState)
+{
+    Q_UNUSED(oldState);
+    if (newState == QAbstractAnimation::Stopped) {
+        m_smoothScrolling = false;
+    }
+}
+
+
 void KItemListContainer::scrollTo(qreal offset)
 {
     const KItemListView* view = m_controller->view();
@@ -231,6 +243,7 @@ void KItemListContainer::scrollTo(qreal offset)
         return;
     }
 
+    m_smoothScrolling = true;
     QScrollBar* scrollBar = (view->scrollOrientation() == Qt::Vertical)
                             ? verticalScrollBar() : horizontalScrollBar();
     scrollBar->setValue(offset);
@@ -261,7 +274,7 @@ void KItemListContainer::updateScrollBars()
 
     const int value = view->offset();
     const int maximum = qMax(0, int(view->maximumOffset() - pageStep));
-    if (m_viewOffsetAnimation->state() == QAbstractAnimation::Running) {
+    if (m_smoothScrollingAnimation->state() == QAbstractAnimation::Running) {
         if (maximum == scrollBar->maximum()) {
             // The value has been changed by the animation, no update
             // of the scrollbars is required as their target state will be
@@ -272,7 +285,7 @@ void KItemListContainer::updateScrollBars()
         // The maximum has been changed which indicates that the content
         // of the view has been changed. Stop the animation in any case and
         // update the scrollbars immediately.
-        m_viewOffsetAnimation->stop();
+        m_smoothScrollingAnimation->stop();
     }
 
     scrollBar->setSingleStep(singleStep);
@@ -324,8 +337,10 @@ void KItemListContainer::initialize()
     QGraphicsView* graphicsView = new KItemListContainerViewport(new QGraphicsScene(this), this);
     setViewport(graphicsView);
 
-    m_viewOffsetAnimation = new QPropertyAnimation(this, "offset");
-    m_viewOffsetAnimation->setDuration(300);
+    m_smoothScrollingAnimation = new QPropertyAnimation(this, "offset");
+    m_smoothScrollingAnimation->setDuration(300);
+    connect(m_smoothScrollingAnimation, SIGNAL(stateChanged(QAbstractAnimation::State,QAbstractAnimation::State)),
+            this, SLOT(slotAnimationStateChanged(QAbstractAnimation::State,QAbstractAnimation::State)));
 
     horizontalScrollBar()->installEventFilter(this);
     verticalScrollBar()->installEventFilter(this);
diff --git a/src/kitemviews/kitemlistcontainer.h b/src/kitemviews/kitemlistcontainer.h
index 0cda436d0..5f846a9bf 100644
--- a/src/kitemviews/kitemlistcontainer.h
+++ b/src/kitemviews/kitemlistcontainer.h
@@ -25,6 +25,7 @@
 
 #include <libdolphin_export.h>
 
+#include <QAbstractAnimation>
 #include <QAbstractScrollArea>
 
 class KItemListController;
@@ -59,6 +60,7 @@ protected:
 private slots:
     void slotModelChanged(KItemModelBase* current, KItemModelBase* previous);
     void slotViewChanged(KItemListView* current, KItemListView* previous);
+    void slotAnimationStateChanged(QAbstractAnimation::State newState, QAbstractAnimation::State oldState);
     void scrollTo(qreal offset);
     void updateScrollBars();
 
@@ -69,8 +71,8 @@ private:
 private:
     KItemListController* m_controller;
 
-    bool m_sliderMovedByUser;
-    QPropertyAnimation* m_viewOffsetAnimation;
+    bool m_smoothScrolling;
+    QPropertyAnimation* m_smoothScrollingAnimation;
 };
 
 #endif
diff --git a/src/kitemviews/kitemlistrubberband.cpp b/src/kitemviews/kitemlistrubberband.cpp
index b8605a7a1..c1f276cdf 100644
--- a/src/kitemviews/kitemlistrubberband.cpp
+++ b/src/kitemviews/kitemlistrubberband.cpp
@@ -64,11 +64,6 @@ void KItemListRubberBand::setActive(bool active)
     if (m_active != active) {
         m_active = active;
         emit activationChanged(active);
-
-        if (!active) {
-            m_startPos = QPointF();
-            m_endPos = QPointF();
-        }
     }
 }
 
diff --git a/src/kitemviews/kitemlistview.cpp b/src/kitemviews/kitemlistview.cpp
index 3b6b4e2e9..f6cfed984 100644
--- a/src/kitemviews/kitemlistview.cpp
+++ b/src/kitemviews/kitemlistview.cpp
@@ -678,7 +678,9 @@ void KItemListView::slotCurrentChanged(int current, int previous)
             }
         }
 
-        emit scrollTo(newOffset);
+        if (newOffset != offset()) {
+            emit scrollTo(newOffset);
+        }
     }
 }
 
@@ -749,7 +751,11 @@ void KItemListView::slotRubberBandStartPosChanged()
 
 void KItemListView::slotRubberBandEndPosChanged()
 {
-    triggerAutoScrolling();
+    // The autoscrolling is triggered asynchronously otherwise it
+    // might be possible to have an endless recursion: The autoscrolling
+    // might adjust the position which might result in updating the
+    // rubberband end-position.
+    QTimer::singleShot(0, this, SLOT(triggerAutoScrolling()));
     update();
 }
 
@@ -766,6 +772,24 @@ void KItemListView::slotRubberBandActivationChanged(bool active)
     update();
 }
 
+void KItemListView::triggerAutoScrolling()
+{
+    int pos = 0;
+    int visibleSize = 0;
+    if (scrollOrientation() == Qt::Vertical) {
+        pos = m_mousePos.y();
+        visibleSize = size().height();
+    } else {
+        pos = m_mousePos.x();
+        visibleSize = size().width();
+    }
+
+    const int inc = calculateAutoScrollingIncrement(pos, visibleSize);
+    if (inc != 0) {
+        emit scrollTo(offset() + inc);
+    }
+}
+
 void KItemListView::setController(KItemListController* controller)
 {
     if (m_controller != controller) {
@@ -1172,15 +1196,6 @@ void KItemListView::updateWidgetProperties(KItemListWidget* widget, int index)
     widget->setData(m_model->data(index));
 }
 
-void KItemListView::triggerAutoScrolling()
-{
-    const int pos = (scrollOrientation() == Qt::Vertical) ? m_mousePos.y() : m_mousePos.x();
-    const int inc = calculateAutoScrollingIncrement(pos, size().height());
-    if (inc != 0) {
-        emit scrollTo(offset() + inc);
-    }
-}
-
 int KItemListView::calculateAutoScrollingIncrement(int pos, int size)
 {
     int inc = 0;
diff --git a/src/kitemviews/kitemlistview.h b/src/kitemviews/kitemlistview.h
index e874957de..1ff56babd 100644
--- a/src/kitemviews/kitemlistview.h
+++ b/src/kitemviews/kitemlistview.h
@@ -191,6 +191,12 @@ private slots:
     void slotRubberBandEndPosChanged();
     void slotRubberBandActivationChanged(bool active);
 
+    /**
+     * Emits the signal scrollTo() with the corresponding target offset if the current
+     * mouse position is above the autoscroll-margin.
+     */
+    void triggerAutoScrolling();
+
 private:
     enum LayoutAnimationHint
     {
@@ -255,12 +261,6 @@ private:
     void updateWidgetProperties(KItemListWidget* widget, int index);
 
     /**
-     * Emits the signal scrollTo() with the corresponding target offset if the current
-     * mouse position is above the autoscroll-margin.
-     */
-    void triggerAutoScrolling();
-
-    /**
      * Helper function for triggerAutoScrolling(). Returns the scroll increment
      * that should be added to the offset() based on the available size \a size
      * and the current mouse position \a pos. As soon as \a pos is inside
diff --git a/src/views/dolphinview.h b/src/views/dolphinview.h
index 12683dbbb..437f12f39 100644
--- a/src/views/dolphinview.h
+++ b/src/views/dolphinview.h
@@ -436,7 +436,7 @@ signals:
      * Is emitted if the view mode (IconsView, DetailsView,
      * PreviewsView) has been changed.
      */
-    void modeChanged(Mode current, Mode previous);
+    void modeChanged(DolphinView::Mode current, DolphinView::Mode previous);
 
     /** Is emitted if the 'show preview' property has been changed. */
     void previewsShownChanged(bool shown);