From kfm-devel Thu Jun 13 20:22:15 2013 From: "Frank Reininghaus" Date: Thu, 13 Jun 2013 20:22:15 +0000 To: kfm-devel Subject: Review Request 111004: When switching to "Sort by Type", really do sort by type Message-Id: <20130613202215.10439.17807 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kfm-devel&m=137115495411900 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============3014136884064404618==" --===============3014136884064404618== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111004/ ----------------------------------------------------------- Review request for Dolphin. Description ------- Emmanuel looked into this issue already in https://git.reviewboard.kde.org/r/109471/, but with this patch, the issue was for some reason not fixed on my system. But I think I have found the root cause of the bug now. When switching to "Sort by Type", KFileItemModel::onSortRoleChanged() resorts the items, but it doesn't work because the "type" role isn't known yet. It only gets added later to the model when KFileItemListView::slotSortRoleChanged() calls applyRolesToModel(), which calls KFileItemModel::setRoles(const QSet& roles) This function does set the "type" role (via retrieveData() - note that item.isMimeTypeKnown() is true because KFileItemModelRolesUpdater has determined the mime type in the mean time), but it does not trigger a resorting. Fixed by adding the new role in KFileItemModel::onSortRoleChanged(), *before* the resorting is triggered. This addresses bugs 310705 and 312014. http://bugs.kde.org/show_bug.cgi?id=310705 http://bugs.kde.org/show_bug.cgi?id=312014 Diffs ----- dolphin/src/tests/kfileitemmodeltest.cpp e636bcd dolphin/src/kitemviews/kfileitemmodel.cpp c78fdc3 Diff: http://git.reviewboard.kde.org/r/111004/diff/ Testing ------- Works for me, old unit tests pass (I had to modify one of them slightly though), added a new test. Thanks, Frank Reininghaus --===============3014136884064404618== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111004/

Review request for Dolphin.
By Frank Reininghaus.

Description 

Emmanuel looked into this issue already in https://git.reviewboard.kde.org/r/109471/, but with this patch, the issue was for some reason not fixed on my system.

But I think I have found the root cause of the bug now. When switching to "Sort by Type", KFileItemModel::onSortRoleChanged() resorts the items, but it doesn't work because the "type" role isn't known yet. It only gets added later to the model when KFileItemListView::slotSortRoleChanged() calls applyRolesToModel(), which calls

KFileItemModel::setRoles(const QSet<QByteArray>& roles)

This function does set the "type" role (via retrieveData() - note that item.isMimeTypeKnown() is true because KFileItemModelRolesUpdater has determined the mime type in the mean time), but it does not trigger a resorting.

Fixed by adding the new role in KFileItemModel::onSortRoleChanged(), *before* the resorting is triggered.

Testing 

Works for me, old unit tests pass (I had to modify one of them slightly though), added a new test.
Bugs: 310705, 312014

Diffs

  • dolphin/src/tests/kfileitemmodeltest.cpp (e636bcd)
  • dolphin/src/kitemviews/kfileitemmodel.cpp (c78fdc3)

View Diff
--===============3014136884064404618==--