Revert "Avoid implicitly selecting items"

This reverts commit 122fee5625f0285ec4ebda79162c72390989eb2a.

This behavior change was well-intentioned, but has significant usability
and speed drawbacks that have not been addressed:
- Keyboard-driven folder manipulation became slower
- Unexpected behavioral differences when opening files with the pointer
  compared to when opening them with the keyboard introduced
  inconsistency and cognitive load
- Unexpected opening of selection mode during fast operation introduced
  the potential for confusion and additional errors
- Dolphin's behavior became inconsistent with that of other file
  managers users may be accustomed to

The bug tracker, discuss.kde.org, and Reddit are full of complaints
about this change. It's been a year now; I think it's clear that many
Dolphin users have not gotten used to and accepted it. I have to count
myself as one of them, I'm afraid. I've tried to get used to it for a
year, but I just have not been able to.

I don't believe the benefits of this change outweigh the drawbacks, so
let's revert it.

24d859cf19e90fa22ed687b36a68231625c1bd80 was explicitly mentioned as a
thing that should also be reverted in this case, but it's already been
done.

BUG: 494125
BUG: 511966
CCBUG: 424723
CCBUG: 492404
FIXED-IN: 25.12.0

4 files changed, 6 insertions, 28 deletions

diff --git a/src/dolphinviewcontainer.cpp b/src/dolphinviewcontainer.cpp
index 0605cb0c8..f6fd01de2 100644
--- a/src/dolphinviewcontainer.cpp
+++ b/src/dolphinviewcontainer.cpp
@@ -912,8 +912,7 @@ void DolphinViewContainer::slotUrlNavigatorLocationChanged(const QUrl &url)
 
 void DolphinViewContainer::slotUrlSelectionRequested(const QUrl &url)
 {
-    // We do not want to select any item here because there is no reason to assume that the user wants to edit the folder we are emerging from. BUG: 424723
-
+    m_view->markUrlsAsSelected({url});
     m_view->markUrlAsCurrent(url); // makes the item scroll into view
 }
 
diff --git a/src/kitemviews/kitemlistcontroller.cpp b/src/kitemviews/kitemlistcontroller.cpp
index 7971de5b7..d889deb4e 100644
--- a/src/kitemviews/kitemlistcontroller.cpp
+++ b/src/kitemviews/kitemlistcontroller.cpp
@@ -732,9 +732,6 @@ bool KItemListController::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event,
 
     const bool emitItemActivated = index.has_value() && index.value() < m_model->count() && !m_view->isAboveExpansionToggle(index.value(), pos);
     if (emitItemActivated) {
-        if (!QApplication::keyboardModifiers()) {
-            m_selectionManager->clearSelection(); // The user does not want to manage/manipulate the item currently, only activate it.
-        }
         Q_EMIT itemActivated(index.value());
     }
     return false;
@@ -1674,14 +1671,10 @@ bool KItemListController::onPress(const QPointF &pos, const Qt::KeyboardModifier
                 // or we just keep going for double-click activation
                 if (singleClickActivation || m_singleClickActivationEnforced) {
                     if (!pressedItemAlreadySelected) {
-                        // An unselected item was clicked directly while deselecting multiple other items so we mark it "current".
+                        // An unselected item was clicked directly while deselecting multiple other items so we select it.
+                        m_selectionManager->setSelected(m_pressedIndex.value(), 1, KItemListSelectionManager::Toggle);
                         m_selectionManager->setCurrentItem(m_pressedIndex.value());
                         m_selectionManager->beginAnchoredSelection(m_pressedIndex.value());
-                        if (!leftClick) {
-                            // We select the item here because this press is not meant to directly activate the item.
-                            // We do not want to select items unless the user wants to edit them.
-                            m_selectionManager->setSelected(m_pressedIndex.value(), 1, KItemListSelectionManager::Toggle);
-                        }
                     }
                     if (leftClick) {
                         row->setPressed(true);
@@ -1756,10 +1749,7 @@ bool KItemListController::onPress(const QPointF &pos, const Qt::KeyboardModifier
             break;
 
         case SingleSelection:
-            if (!leftClick || shiftOrControlPressed || (!singleClickActivation && !m_singleClickActivationEnforced)) {
-                m_selectionManager->setSelected(m_pressedIndex.value());
-            }
-            break;
+            m_selectionManager->setSelected(m_pressedIndex.value());
 
         case MultiSelection:
             if (controlPressed && !shiftPressed && leftClick) {
@@ -1778,10 +1768,7 @@ bool KItemListController::onPress(const QPointF &pos, const Qt::KeyboardModifier
                     // This will be the start of an item drag-to-copy operation if the user now moves the mouse before releasing the mouse button.
                 }
             } else if (!shiftPressed || !m_selectionManager->isAnchoredSelectionActive()) {
-                // Select the pressed item and start a new anchored selection
-                if (!leftClick || shiftOrControlPressed || (!singleClickActivation && !m_singleClickActivationEnforced)) {
-                    m_selectionManager->setSelected(m_pressedIndex.value(), 1, KItemListSelectionManager::Select);
-                }
+                m_selectionManager->setSelected(m_pressedIndex.value(), 1, KItemListSelectionManager::Select);
                 m_selectionManager->beginAnchoredSelection(m_pressedIndex.value());
             }
             break;
diff --git a/src/tests/dolphinmainwindowtest.cpp b/src/tests/dolphinmainwindowtest.cpp
index d309b6d9c..fb3c95383 100644
--- a/src/tests/dolphinmainwindowtest.cpp
+++ b/src/tests/dolphinmainwindowtest.cpp
@@ -672,7 +672,7 @@ void DolphinMainWindowTest::testGoActions()
     };
 
     QCOMPARE(currentItemUrl(), childDirUrl); // The item we just emerged from should now have keyboard focus.
-    QCOMPARE(m_mainWindow->m_activeViewContainer->view()->selectedItems().count(), 0); // The item we just emerged from should not be selected. BUG: 424723
+    QCOMPARE(m_mainWindow->m_activeViewContainer->view()->selectedItems().count(), 1); // …and it should be selected, too.
     // Pressing arrow keys should not only move the keyboard focus but also select the item.
     // We press "Down" to select "c" below and then "Up" so the folder "b" we just emerged from is selected for the first time.
     m_mainWindow->actionCollection()->action(QStringLiteral("compact"))->trigger();
diff --git a/src/tests/kitemlistcontrollertest.cpp b/src/tests/kitemlistcontrollertest.cpp
index e9e1e66c2..8fd0f6158 100644
--- a/src/tests/kitemlistcontrollertest.cpp
+++ b/src/tests/kitemlistcontrollertest.cpp
@@ -1088,7 +1088,6 @@ void KItemListControllerTest::testMouseClickActivation()
     m_view->event(&mouseReleaseEvent);
     QCOMPARE(spyItemActivated.count(), 1);
     spyItemActivated.clear();
-    QVERIFY2(!m_view->controller()->selectionManager()->hasSelection(), "An item should not be implicitly selected during activation. @see bug 424723");
 
     // Set the global setting to "double click activation".
     m_testStyle->setActivateItemOnSingleClick(false);
@@ -1096,7 +1095,6 @@ void KItemListControllerTest::testMouseClickActivation()
     m_view->event(&mouseReleaseEvent);
     QCOMPARE(spyItemActivated.count(), 0);
     spyItemActivated.clear();
-    QVERIFY(m_view->controller()->selectionManager()->hasSelection());
 
     // emulation of double click according to https://doc.qt.io/qt-6/qgraphicsscene.html#mouseDoubleClickEvent
     m_view->event(&mousePressEvent);
@@ -1105,7 +1103,6 @@ void KItemListControllerTest::testMouseClickActivation()
     m_view->event(&mouseReleaseEvent);
     QCOMPARE(spyItemActivated.count(), 1);
     spyItemActivated.clear();
-    QVERIFY2(!m_view->controller()->selectionManager()->hasSelection(), "An item should not be implicitly selected during activation. @see bug 424723");
 
     // right mouse button should not trigger activation
     m_view->event(&mouseRightPressEvent);
@@ -1127,8 +1124,6 @@ void KItemListControllerTest::testMouseClickActivation()
     m_view->event(&mouseReleaseEvent);
     QCOMPARE(spyItemActivated.count(), 1);
     spyItemActivated.clear();
-    constexpr const char *reasonWhySelectionShouldPersist = "An item was selected before this mouse click. The click should not have cleared this selection.";
-    QVERIFY2(m_view->controller()->selectionManager()->hasSelection(), reasonWhySelectionShouldPersist);
 
     // Do not enforce single click activation in the controller.
     m_controller->setSingleClickActivationEnforced(false);
@@ -1136,7 +1131,6 @@ void KItemListControllerTest::testMouseClickActivation()
     m_view->event(&mouseReleaseEvent);
     QCOMPARE(spyItemActivated.count(), 0);
     spyItemActivated.clear();
-    QVERIFY2(m_view->controller()->selectionManager()->hasSelection(), reasonWhySelectionShouldPersist);
 
     // Set the global setting back to "single click activation".
     m_testStyle->setActivateItemOnSingleClick(true);
@@ -1144,7 +1138,6 @@ void KItemListControllerTest::testMouseClickActivation()
     m_view->event(&mouseReleaseEvent);
     QCOMPARE(spyItemActivated.count(), 1);
     spyItemActivated.clear();
-    QVERIFY2(m_view->controller()->selectionManager()->hasSelection(), reasonWhySelectionShouldPersist);
 
     // Enforce single click activation in the controller.
     m_controller->setSingleClickActivationEnforced(true);
@@ -1152,7 +1145,6 @@ void KItemListControllerTest::testMouseClickActivation()
     m_view->event(&mouseReleaseEvent);
     QCOMPARE(spyItemActivated.count(), 1);
     spyItemActivated.clear();
-    QVERIFY2(m_view->controller()->selectionManager()->hasSelection(), reasonWhySelectionShouldPersist);
 
     // Restore previous settings.
     m_controller->setSingleClickActivationEnforced(true);