From a4efbfbfa69f04ed21fe703a11bb59416ef8a821 Mon Sep 17 00:00:00 2001 From: Felix Ernst Date: Fri, 26 Jan 2024 19:12:23 +0100 Subject: [PATCH] Fix focus chain Prior to this commit pressing Tab repeatedly would bring the focus to the end of the status bar but not further. This commit makes sure the tab focus doesn't get stuck on the invisible tab bar by explicitly removing the DolphinTabBar from the focus chain while it is hidden. I don't understand why pressing Tab doesn't do anything for the invisible tab bar, but removing an invisible and currently useless widget from the focus chain seems sensible in any case. Improve the accessibility autotest to prevent regressions concerning this. --- src/dolphintabbar.cpp | 25 +++++++++++++ src/tests/dolphinmainwindowtest.cpp | 54 +++++++++++++++++++---------- 2 files changed, 61 insertions(+), 18 deletions(-) diff --git a/src/dolphintabbar.cpp b/src/dolphintabbar.cpp index 55b5e5edfd..4c918e6114 100644 --- a/src/dolphintabbar.cpp +++ b/src/dolphintabbar.cpp @@ -13,6 +13,28 @@ #include #include +class PreventFocusWhileHidden : public QObject +{ +public: + PreventFocusWhileHidden(QObject *parent) + : QObject(parent){}; + +protected: + bool eventFilter(QObject *obj, QEvent *ev) override + { + switch (ev->type()) { + case QEvent::Hide: + static_cast(obj)->setFocusPolicy(Qt::NoFocus); + return false; + case QEvent::Show: + static_cast(obj)->setFocusPolicy(Qt::TabFocus); + return false; + default: + return false; + } + }; +}; + DolphinTabBar::DolphinTabBar(QWidget *parent) : QTabBar(parent) , m_autoActivationIndex(-1) @@ -23,6 +45,9 @@ DolphinTabBar::DolphinTabBar(QWidget *parent) setMovable(true); setTabsClosable(true); + setFocusPolicy(Qt::NoFocus); + installEventFilter(new PreventFocusWhileHidden(this)); + m_autoActivationTimer = new QTimer(this); m_autoActivationTimer->setSingleShot(true); m_autoActivationTimer->setInterval(800); diff --git a/src/tests/dolphinmainwindowtest.cpp b/src/tests/dolphinmainwindowtest.cpp index f106910134..6b4b0f71b7 100644 --- a/src/tests/dolphinmainwindowtest.cpp +++ b/src/tests/dolphinmainwindowtest.cpp @@ -539,29 +539,47 @@ void DolphinMainWindowTest::testAccessibilityAncestorTree() QVERIFY(QTest::qWaitForWindowExposed(m_mainWindow.data())); QVERIFY(m_mainWindow->isVisible()); - std::set testedObjects; // Makes sure we stop testing if we arrive at an item that was already tested. QAccessibleInterface *accessibleInterfaceOfMainWindow = QAccessible::queryAccessibleInterface(m_mainWindow.get()); Q_CHECK_PTR(accessibleInterfaceOfMainWindow); - // We will do accessibility checks for every object that gets focus. Focus will be changed using the Tab key. - while (qApp->focusObject() && !testedObjects.count(qApp->focusObject())) { - const auto currentlyFocusedObject = qApp->focusObject(); - QAccessibleInterface *accessibleInterface = QAccessible::queryAccessibleInterface(currentlyFocusedObject); - - // The accessibleInterfaces of focused objects might themselves have children. - // We go down that hierarchy as far as possible and then test the ancestor tree from there. - while (accessibleInterface->childCount() > 0) { - accessibleInterface = accessibleInterface->child(0); - } - while (accessibleInterface != accessibleInterfaceOfMainWindow) { - QVERIFY2(accessibleInterface, - qPrintable(QString("%1's accessibleInterface or one of its accessible children doesn't have the main window as an ancestor.") - .arg(currentlyFocusedObject->metaObject()->className()))); - accessibleInterface = accessibleInterface->parent(); + // We will test the accessibility of objects traversing forwards and backwards. + int testedObjectsSizeAfterTraversingForwards = 0; + for (int i = 0; i < 2; i++) { + std::tuple focusChainTraversalKeyCombination = {Qt::Key::Key_Tab, Qt::NoModifier}; + if (i) { + focusChainTraversalKeyCombination = {Qt::Key::Key_Tab, Qt::ShiftModifier}; } - testedObjects.insert(currentlyFocusedObject); // Add it to testedObjects so we won't test it again later. - QTest::keyClick(m_mainWindow.get(), Qt::Key::Key_Tab, Qt::ShiftModifier); // ShiftModifier because the Tab cycle is currently broken going forward. + // We will do accessibility checks for every object that gets focus. Focus will be changed using the focusChainTraversalKeyCombination. + std::set testedObjects; // Makes sure we stop testing when we arrive at an item that was already tested. + while (qApp->focusObject() && !testedObjects.count(qApp->focusObject())) { + const auto currentlyFocusedObject = qApp->focusObject(); + + QAccessibleInterface *accessibleInterface = QAccessible::queryAccessibleInterface(currentlyFocusedObject); + // The accessibleInterfaces of focused objects might themselves have children. + // We go down that hierarchy as far as possible and then test the ancestor tree from there. + while (accessibleInterface->childCount() > 0) { + accessibleInterface = accessibleInterface->child(0); + } + while (accessibleInterface != accessibleInterfaceOfMainWindow) { + QVERIFY2(accessibleInterface, + qPrintable(QString("%1's accessibleInterface or one of its accessible children doesn't have the main window as an ancestor.") + .arg(currentlyFocusedObject->metaObject()->className()))); + accessibleInterface = accessibleInterface->parent(); + } + + testedObjects.insert(currentlyFocusedObject); // Add it to testedObjects so we won't test it again later. + QTest::keyClick(m_mainWindow.get(), std::get<0>(focusChainTraversalKeyCombination), std::get<1>(focusChainTraversalKeyCombination)); + QVERIFY2(currentlyFocusedObject != qApp->focusObject(), + "The focus chain is broken. The focused object should have changed after pressing the focusChainTraversalKeyCombination."); + } + + if (i == 0) { + testedObjectsSizeAfterTraversingForwards = testedObjects.size(); + } else { + QCOMPARE(testedObjects.size(), testedObjectsSizeAfterTraversingForwards); // The size after traversing backwards is different than + // after going forwards which is probably not intended. + } } }