D17042: Fix selection when navigating back, with size sorting.

[Thomas Surrel]

thsurrel created this revision.
thsurrel added a reviewer: Dolphin.
Herald added a project: Dolphin.
Herald added a subscriber: kfm-devel.
thsurrel requested review of this revision.

REVISION SUMMARY
KItemListSelectionManager::itemsMoved (called when sorting by size)
was re-activating anchor selection regardless if we actually were
doing an anchored selection. This was leading to an incorrect
selection when navigating back.

BUG: 352296

TEST PLAN
In any folder, sort by size then move to a subfolder. Navigate back
to the parent folder: only the parent folder should be selected.

REPOSITORY
R318 Dolphin

BRANCH
arc_back_selection (branched from master)

REVISION DETAIL
https://phabricator.kde.org/D17042

AFFECTED FILES
src/kitemviews/kitemlistselectionmanager.cpp

To: thsurrel, #dolphin
Cc: kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Thomas Surrel]

thsurrel updated this revision to Diff 45876.
thsurrel added a comment.


Fix removed empty line

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17042?vs=45875&id=45876

BRANCH
arc_back_selection (branched from master)

REVISION DETAIL
https://phabricator.kde.org/D17042

AFFECTED FILES
src/kitemviews/kitemlistselectionmanager.cpp

To: thsurrel, #dolphin
Cc: kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Kai Uwe Broulik]

broulik added a comment.


Nice! I've always wondered why that happens but couldn't reliably reproduce it

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D17042

To: thsurrel, #dolphin
Cc: broulik, kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Elvis Angelaccio]

elvisangelaccio added a comment.


Thanks, patch looks good and `kitemlistselectionmanagertest` still passes.

I don't understand though why the bug only happens with size sorting...

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D17042

To: thsurrel, #dolphin
Cc: elvisangelaccio, broulik, kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Thomas Surrel]

thsurrel added a comment.


That happens only with size sorting because we modify the "size" of folders by setting the number of items in them. That triggers a re-sort once this is done, that it turns call itemsMoved and triggers the bug.
See the special case in KFileItemModelRolesUpdater::applySortRole for details.

It took me a while to figure this out, but it looks like there is no other root cause to this bug.

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D17042

To: thsurrel, #dolphin
Cc: elvisangelaccio, broulik, kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Elvis Angelaccio]

elvisangelaccio accepted this revision.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


I think we can land this on the stable branch too (`Applications/18.12`).

REPOSITORY
R318 Dolphin

BRANCH
arc_back_selection (branched from master)

REVISION DETAIL
https://phabricator.kde.org/D17042

To: thsurrel, #dolphin, elvisangelaccio
Cc: elvisangelaccio, broulik, kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Thomas Surrel]

This revision was automatically updated to reflect the committed changes.
Closed by commit R318:6100f66ae2ba: Fix selection when navigating back, with size sorting. (authored by thsurrel).

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17042?vs=45876&id=46654

REVISION DETAIL
https://phabricator.kde.org/D17042

AFFECTED FILES
src/kitemviews/kitemlistselectionmanager.cpp

To: thsurrel, #dolphin, elvisangelaccio
Cc: elvisangelaccio, broulik, kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov