Include "Space" in the keyboard search string

Before this commit, we only added pressed keys to the search string if
they have no other meaning. This means that files containing a Space in
their name could not be searched because Ctrl+Space toggles the
selection state of the current item, and Space alone selects the
current item.

After this commit, Space is added to the search string if

(a) the key press did not have any other effect, i.e., if Ctrl was not
    pressed, and the current item is selected already, and
(b) a keyboard search has been started already (to prevent unexpected
    effects when pressing Space accidentally - I think that it's rather
    uncommon to have files whose names start with a Space - and to make
    the unit test simpler).

I modified the unit test of KItemListController, which did not test
keyboard search yet. This uncovered a small problem in
KItemListController::slotChangeCurrentItem() when NoSelection mode is
used. It's not really relevant for anything that is executed inside
Dolphin, but I still fixed it to make the unit test happy.

BUG: 324479
FIXED-IN: 4.11.3
REVIEW: 113071

diff --git a/src/kitemviews/kitemlistcontroller.cpp b/src/kitemviews/kitemlistcontroller.cpp
index 4629b29f1b3a13f30e1a1ef731a786a0b6a73607..9335ab816c334631ffa53ff6cbf78563e9399097 100644 (file)
--- a/src/kitemviews/kitemlistcontroller.cpp
+++ b/src/kitemviews/kitemlistcontroller.cpp
@@ -375,19 +375,6 @@ bool KItemListController::keyPressEvent(QKeyEvent* event)
         break;
     }
 
         break;
     }
 

-    case Qt::Key_Space:
-        if (m_selectionBehavior == MultiSelection) {
-            if (controlPressed) {
-                m_selectionManager->endAnchoredSelection();
-                m_selectionManager->setSelected(index, 1, KItemListSelectionManager::Toggle);
-                m_selectionManager->beginAnchoredSelection(index);
-            } else {
-                const int current = m_selectionManager->currentItem();
-                m_selectionManager->setSelected(current);
-            }
-        }
-        break;
-

     case Qt::Key_Menu: {
         // Emit the signal itemContextMenuRequested() in case if at least one
         // item is selected. Otherwise the signal viewContextMenuRequested() will be emitted.
     case Qt::Key_Menu: {
         // Emit the signal itemContextMenuRequested() in case if at least one
         // item is selected. Otherwise the signal viewContextMenuRequested() will be emitted.


@@ -418,6 +405,25 @@ bool KItemListController::keyPressEvent(QKeyEvent* event)
         m_keyboardManager->cancelSearch();
         break;
 
         m_keyboardManager->cancelSearch();
         break;
 

+    case Qt::Key_Space:
+        if (m_selectionBehavior == MultiSelection) {
+            if (controlPressed) {
+                // Toggle the selection state of the current item.
+                m_selectionManager->endAnchoredSelection();
+                m_selectionManager->setSelected(index, 1, KItemListSelectionManager::Toggle);
+                m_selectionManager->beginAnchoredSelection(index);
+                break;
+            } else {
+                // Select the current item if it is not selected yet.
+                const int current = m_selectionManager->currentItem();
+                if (!m_selectionManager->isSelected(current)) {
+                    m_selectionManager->setSelected(current);
+                    break;
+                }
+            }
+        }
+        // Fall through to the default case and add the Space to the current search string.
+

     default:
         m_keyboardManager->addKeys(event->text());
         // Make sure unconsumed events get propagated up the chain. #302329
     default:
         m_keyboardManager->addKeys(event->text());
         // Make sure unconsumed events get propagated up the chain. #302329


@@ -474,9 +480,13 @@ void KItemListController::slotChangeCurrentItem(const QString& text, bool search
     }
     if (index >= 0) {
         m_selectionManager->setCurrentItem(index);
     }
     if (index >= 0) {
         m_selectionManager->setCurrentItem(index);

-        m_selectionManager->clearSelection();
-        m_selectionManager->setSelected(index, 1);
-        m_selectionManager->beginAnchoredSelection(index);
+
+        if (m_selectionBehavior != NoSelection) {
+            m_selectionManager->clearSelection();
+            m_selectionManager->setSelected(index, 1);
+            m_selectionManager->beginAnchoredSelection(index);
+        }
+

         m_view->scrollToItem(index);
     }
 }
         m_view->scrollToItem(index);
     }
 }

diff --git a/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp b/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp
index 38154864bba558eed3bbb0435eb3feda31c1f39c..3bd1b01f38c9757deb18f057e949deab34fe6f15 100644 (file)
--- a/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp
+++ b/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp
@@ -46,6 +46,12 @@ void KItemListKeyboardSearchManager::addKeys(const QString& keys)
 
     const bool newSearch = m_searchedString.isEmpty();
 
 
     const bool newSearch = m_searchedString.isEmpty();
 

+    // Do not start a new search if the user pressed Space. Only add
+    // it to the search string if a search is in progress already.
+    if (newSearch && keys == QLatin1String(" ")) {
+        return;
+    }
+

     if (!keys.isEmpty()) {
         m_searchedString.append(keys);
 
     if (!keys.isEmpty()) {
         m_searchedString.append(keys);
 

diff --git a/src/tests/kitemlistcontrollertest.cpp b/src/tests/kitemlistcontrollertest.cpp
index d8f838873766bd26aa64fd5a47e0399523394b98..60e93e539abc9f87926ba7c9c0dcbf36440a4569 100644 (file)
--- a/src/tests/kitemlistcontrollertest.cpp
+++ b/src/tests/kitemlistcontrollertest.cpp
@@ -98,7 +98,7 @@ void KItemListControllerTest::initTestCase()
         << "b1"
         << "c1" << "c2" << "c3" << "c4" << "c5"
         << "d1" << "d2" << "d3" << "d4"
         << "b1"
         << "c1" << "c2" << "c3" << "c4" << "c5"
         << "d1" << "d2" << "d3" << "d4"

-        << "e1" << "e2" << "e3" << "e4" << "e5" << "e6" << "e7";
+        << "e" << "e 2" << "e 3" << "e 4" << "e 5" << "e 6" << "e 7";

 
     m_testDir->createFiles(files);
     m_model->loadDirectory(m_testDir->url());
 
     m_testDir->createFiles(files);
     m_model->loadDirectory(m_testDir->url());


@@ -282,7 +282,14 @@ void KItemListControllerTest::testKeyboardNavigation_data()
                         << qMakePair(KeyPress(Qt::Key_Home), ViewState(0, QSet<int>() << 0))
                         << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet<int>()))
                         << qMakePair(KeyPress(Qt::Key_Enter), ViewState(0, QSet<int>(), true))
                         << qMakePair(KeyPress(Qt::Key_Home), ViewState(0, QSet<int>() << 0))
                         << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet<int>()))
                         << qMakePair(KeyPress(Qt::Key_Enter), ViewState(0, QSet<int>(), true))

-                        << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet<int>() << 0));
+                        << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet<int>() << 0))
+                        << qMakePair(KeyPress(Qt::Key_Space, Qt::ControlModifier), ViewState(0, QSet<int>()))
+                        << qMakePair(KeyPress(Qt::Key_Space), ViewState(0, QSet<int>() << 0))
+                        << qMakePair(KeyPress(Qt::Key_E), ViewState(13, QSet<int>() << 13))
+                        << qMakePair(KeyPress(Qt::Key_Space), ViewState(14, QSet<int>() << 14))
+                        << qMakePair(KeyPress(Qt::Key_3), ViewState(15, QSet<int>() << 15))
+                        << qMakePair(KeyPress(Qt::Key_Home), ViewState(0, QSet<int>() << 0))
+                        << qMakePair(KeyPress(Qt::Key_Escape), ViewState(0, QSet<int>()));

 
                     // Next, we test combinations of key presses which only work for a
                     // particular number of columns and either enabled or disabled grouping.
 
                     // Next, we test combinations of key presses which only work for a
                     // particular number of columns and either enabled or disabled grouping.