Fix grouping-issue with not visible sorting roles

It must be assured that the data for a sorting role always is
determined even it is not shown as "additional info" in the view.

6 files changed, 119 insertions, 53 deletions

diff --git a/src/kitemviews/kfileitemlistview.cpp b/src/kitemviews/kfileitemlistview.cpp
index 783af5a26..528140deb 100644
--- a/src/kitemviews/kfileitemlistview.cpp
+++ b/src/kitemviews/kfileitemlistview.cpp
@@ -304,27 +304,9 @@ void KFileItemListView::onScrollOffsetChanged(qreal current, qreal previous)
 
 void KFileItemListView::onVisibleRolesChanged(const QList<QByteArray>& current, const QList<QByteArray>& previous)
 {
+    Q_UNUSED(current);
     Q_UNUSED(previous);
-
-    Q_ASSERT(qobject_cast<KFileItemModel*>(model()));
-    KFileItemModel* fileItemModel = static_cast<KFileItemModel*>(model());
-
-    // KFileItemModel does not distinct between "visible" and "invisible" roles.
-    // Add all roles that are mandatory for having a working KFileItemListView:
-    QSet<QByteArray> keys = current.toSet();
-    QSet<QByteArray> roles = keys;
-    roles.insert("iconPixmap");
-    roles.insert("iconName");
-    roles.insert("name"); // TODO: just don't allow to disable it
-    roles.insert("isDir");
-    if (m_itemLayout == DetailsLayout) {
-        roles.insert("isExpanded");
-        roles.insert("expansionLevel");
-    }
-
-    fileItemModel->setRoles(roles);
-
-    m_modelRolesUpdater->setRoles(keys);
+    applyRolesToModel();
 }
 
 void KFileItemListView::onStyleOptionChanged(const KItemListStyleOption& current, const KItemListStyleOption& previous)
@@ -363,6 +345,16 @@ void KFileItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
     updateTimersInterval();
 }
 
+void KFileItemListView::slotSortRoleChanged(const QByteArray& current, const QByteArray& previous)
+{
+    const QByteArray sortRole = model()->sortRole();
+    if (!visibleRoles().contains(sortRole)) {
+        applyRolesToModel();
+    }
+
+    KItemListView::slotSortRoleChanged(current, previous);
+}
+
 void KFileItemListView::triggerVisibleIndexRangeUpdate()
 {
     m_modelRolesUpdater->setPaused(true);
@@ -481,4 +473,28 @@ void KFileItemListView::updateMinimumRolesWidths()
     m_minimumRolesWidths.insert("size", option.fontMetrics.width(sizeText));
 }
 
+void KFileItemListView::applyRolesToModel()
+{
+    Q_ASSERT(qobject_cast<KFileItemModel*>(model()));
+    KFileItemModel* fileItemModel = static_cast<KFileItemModel*>(model());
+
+    // KFileItemModel does not distinct between "visible" and "invisible" roles.
+    // Add all roles that are mandatory for having a working KFileItemListView:
+    QSet<QByteArray> roles = visibleRoles().toSet();
+    roles.insert("iconPixmap");
+    roles.insert("iconName");
+    roles.insert("name");
+    roles.insert("isDir");
+    if (m_itemLayout == DetailsLayout) {
+        roles.insert("isExpanded");
+        roles.insert("expansionLevel");
+    }
+
+    // Assure that the role that is used for sorting will be determined
+    roles.insert(fileItemModel->sortRole());
+
+    fileItemModel->setRoles(roles);
+    m_modelRolesUpdater->setRoles(roles);
+}
+
 #include "kfileitemlistview.moc"
diff --git a/src/kitemviews/kfileitemlistview.h b/src/kitemviews/kfileitemlistview.h
index 125c86142..d90d8ecf9 100644
--- a/src/kitemviews/kfileitemlistview.h
+++ b/src/kitemviews/kfileitemlistview.h
@@ -81,6 +81,7 @@ protected:
 
 protected slots:
     virtual void slotItemsRemoved(const KItemRangeList& itemRanges);
+    virtual void slotSortRoleChanged(const QByteArray& current, const QByteArray& previous);
 
 private slots:
     void triggerVisibleIndexRangeUpdate();
@@ -95,6 +96,14 @@ private:
     void updateTimersInterval();
     void updateMinimumRolesWidths();
 
+    /**
+     * Applies the roles defined by KItemListView::visibleRoles() to the
+     * KFileItemModel and KFileItemModelRolesUpdater. As the model does not
+     * distinct between visible and invisible roles also internal roles
+     * are applied that are mandatory for having a working KFileItemModel.
+     */
+    void applyRolesToModel();
+
 private:
     Layout m_itemLayout;
 
diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index 083b6421c..60f1c72ce 100644
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -36,6 +36,7 @@ KFileItemModel::KFileItemModel(KDirLister* dirLister, QObject* parent) :
     m_naturalSorting(true),
     m_sortFoldersFirst(true),
     m_sortRole(NameRole),
+    m_roles(),
     m_caseSensitivity(Qt::CaseInsensitive),
     m_sortedItems(),
     m_items(),
@@ -306,6 +307,8 @@ void KFileItemModel::clear()
 
 void KFileItemModel::setRoles(const QSet<QByteArray>& roles)
 {
+    m_roles = roles;
+
     if (count() > 0) {
         const bool supportedExpanding = m_requestRole[IsExpandedRole] && m_requestRole[ExpansionLevelRole];
         const bool willSupportExpanding = roles.contains("isExpanded") && roles.contains("expansionLevel");
@@ -317,6 +320,7 @@ void KFileItemModel::setRoles(const QSet<QByteArray>& roles)
     }
 
     resetRoles();
+
     QSetIterator<QByteArray> it(roles);
     while (it.hasNext()) {
         const QByteArray& role = it.next();
@@ -337,28 +341,7 @@ void KFileItemModel::setRoles(const QSet<QByteArray>& roles)
 
 QSet<QByteArray> KFileItemModel::roles() const
 {
-    QSet<QByteArray> roles;
-    for (int i = 0; i < RolesCount; ++i) {
-        if (m_requestRole[i]) {
-            switch (i) {
-            case NoRole:             break;
-            case NameRole:           roles.insert("name"); break;
-            case SizeRole:           roles.insert("size"); break;
-            case DateRole:           roles.insert("date"); break;
-            case PermissionsRole:    roles.insert("permissions"); break;
-            case OwnerRole:          roles.insert("owner"); break;
-            case GroupRole:          roles.insert("group"); break;
-            case TypeRole:           roles.insert("type"); break;
-            case DestinationRole:    roles.insert("destination"); break;
-            case PathRole:           roles.insert("path"); break;
-            case IsDirRole:          roles.insert("isDir"); break;
-            case IsExpandedRole:     roles.insert("isExpanded"); break;
-            case ExpansionLevelRole: roles.insert("expansionLevel"); break;
-            default:                 Q_ASSERT(false); break;
-            }
-        }
-    }
-    return roles;
+    return m_roles;
 }
 
 bool KFileItemModel::setExpanded(int index, bool expanded)
@@ -435,6 +418,13 @@ void KFileItemModel::onSortRoleChanged(const QByteArray& current, const QByteArr
 {
     Q_UNUSED(previous);
     m_sortRole = roleIndex(current);
+
+#ifdef KFILEITEMMODEL_DEBUG
+    if (!m_requestRole[m_sortRole]) {
+        kWarning() << "The sort-role has been changed to a role that has not been received yet";
+    }
+#endif
+
     resortAllItems();
 }
 
diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h
index f2e783f46..e34503ebb 100644
--- a/src/kitemviews/kfileitemmodel.h
+++ b/src/kitemviews/kfileitemmodel.h
@@ -166,6 +166,9 @@ private:
         RolesCount // Mandatory last entry
     };
 
+    /**
+     * Resets all values from m_requestRole to false.
+     */
     void resetRoles();
 
     Role roleIndex(const QByteArray& role) const;
@@ -217,6 +220,7 @@ private:
     bool m_sortFoldersFirst;
 
     Role m_sortRole;
+    QSet<QByteArray> m_roles;
     Qt::CaseSensitivity m_caseSensitivity;
 
     KFileItemList m_sortedItems;   // Allows O(1) access for KFileItemModel::fileItem(int index)
diff --git a/src/kitemviews/kitemlistview.cpp b/src/kitemviews/kitemlistview.cpp
index eff48cf09..24fb62aef 100644
--- a/src/kitemviews/kitemlistview.cpp
+++ b/src/kitemviews/kitemlistview.cpp
@@ -856,19 +856,17 @@ void KItemListView::slotItemsChanged(const KItemRangeList& itemRanges,
 void KItemListView::slotGroupedSortingChanged(bool current)
 {
     m_grouped = current;
+
     if (m_grouped) {
         // Apply the height of the header to the layouter
         const qreal groupHeaderHeight = m_styleOption.fontMetrics.height() +
                                         m_styleOption.margin * 2;
         m_layouter->setGroupHeaderHeight(groupHeaderHeight);
 
-        // Assure that headers from already visible items get created
-        QHashIterator<int, KItemListWidget*> it(m_visibleItems);
-        while (it.hasNext()) {
-            it.next();
-            updateGroupHeaderForWidget(it.value());
-        }
+        updateVisibleGroupHeaders(); // Triggers updateLayout()
     } else {
+        m_layouter->markAsDirty();
+
         // Clear all visible headers
         QMutableHashIterator<KItemListWidget*, KItemListGroupHeader*> it (m_visibleGroups);
         while (it.hasNext()) {
@@ -876,10 +874,27 @@ void KItemListView::slotGroupedSortingChanged(bool current)
             recycleGroupHeaderForWidget(it.key());
         }
         Q_ASSERT(m_visibleGroups.isEmpty());
+
+        updateLayout();
     }
+}
 
-    m_layouter->markAsDirty();
-    updateLayout();
+void KItemListView::slotSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous)
+{
+    Q_UNUSED(current);
+    Q_UNUSED(previous);
+    if (m_grouped) {
+        updateVisibleGroupHeaders();
+    }
+}
+
+void KItemListView::slotSortRoleChanged(const QByteArray& current, const QByteArray& previous)
+{
+    Q_UNUSED(current);
+    Q_UNUSED(previous);
+    if (m_grouped) {
+        updateVisibleGroupHeaders();
+    }
 }
 
 void KItemListView::slotCurrentChanged(int current, int previous)
@@ -1136,6 +1151,10 @@ void KItemListView::setModel(KItemModelBase* model)
                    this,    SLOT(slotItemsMoved(KItemRange,QList<int>)));
         disconnect(m_model, SIGNAL(groupedSortingChanged(bool)),
                    this,    SLOT(slotGroupedSortingChanged(bool)));
+        disconnect(m_model, SIGNAL(sortOrderChanged(Qt::SortOrder,Qt::SortOrder)),
+                   this,    SLOT(slotSortOrderChanged(Qt::SortOrder,Qt::SortOrder)));
+        disconnect(m_model, SIGNAL(sortRoleChanged(QByteArray,QByteArray)),
+                   this,    SLOT(slotSortRoleChanged(QByteArray,QByteArray)));
     }
 
     m_model = model;
@@ -1153,6 +1172,10 @@ void KItemListView::setModel(KItemModelBase* model)
                 this,    SLOT(slotItemsMoved(KItemRange,QList<int>)));
         connect(m_model, SIGNAL(groupedSortingChanged(bool)),
                 this,    SLOT(slotGroupedSortingChanged(bool)));
+        connect(m_model, SIGNAL(sortOrderChanged(Qt::SortOrder,Qt::SortOrder)),
+                this,    SLOT(slotSortOrderChanged(Qt::SortOrder,Qt::SortOrder)));
+        connect(m_model, SIGNAL(sortRoleChanged(QByteArray,QByteArray)),
+                this,    SLOT(slotSortRoleChanged(QByteArray,QByteArray)));
     }
 
     onModelChanged(model, previous);
@@ -1528,6 +1551,20 @@ void KItemListView::recycleGroupHeaderForWidget(KItemListWidget* widget)
     }
 }
 
+void KItemListView::updateVisibleGroupHeaders()
+{
+    Q_ASSERT(m_grouped);
+    m_layouter->markAsDirty();
+
+    QHashIterator<int, KItemListWidget*> it(m_visibleItems);
+    while (it.hasNext()) {
+        it.next();
+        updateGroupHeaderForWidget(it.value());
+    }
+
+    updateLayout();
+}
+
 QHash<QByteArray, qreal> KItemListView::headerRolesWidths() const
 {
     QHash<QByteArray, qreal> rolesWidths;
diff --git a/src/kitemviews/kitemlistview.h b/src/kitemviews/kitemlistview.h
index fd2f286db..397ecedf5 100644
--- a/src/kitemviews/kitemlistview.h
+++ b/src/kitemviews/kitemlistview.h
@@ -252,10 +252,13 @@ protected slots:
     virtual void slotItemsChanged(const KItemRangeList& itemRanges,
                                   const QSet<QByteArray>& roles);
 
+    virtual void slotGroupedSortingChanged(bool current);
+    virtual void slotSortOrderChanged(Qt::SortOrder current, Qt::SortOrder previous);
+    virtual void slotSortRoleChanged(const QByteArray& current, const QByteArray& previous);
+    virtual void slotCurrentChanged(int current, int previous);
+    virtual void slotSelectionChanged(const QSet<int>& current, const QSet<int>& previous);
+
 private slots:
-    void slotGroupedSortingChanged(bool current);
-    void slotCurrentChanged(int current, int previous);
-    void slotSelectionChanged(const QSet<int>& current, const QSet<int>& previous);
     void slotAnimationFinished(QGraphicsWidget* widget,
                                KItemListViewAnimation::AnimationType type);
     void slotLayoutTimerFinished();
@@ -348,6 +351,13 @@ private:
     void recycleGroupHeaderForWidget(KItemListWidget* widget);
 
     /**
+     * Helper method for slotGroupedSortingChanged(), slotSortOrderChanged()
+     * and slotSortRoleChanged(): Iterates through all visible items and updates
+     * the group-header widgets.
+     */
+    void updateVisibleGroupHeaders();
+
+    /**
      * @return The widths of each visible role that is shown in the KItemListHeader.
      */
     QHash<QByteArray, qreal> headerRolesWidths() const;