From d6bda220f799bd1a779c0e3d8b796b1fd23205c2 Mon Sep 17 00:00:00 2001
From: Volker Hilsheimer <volker.hilsheimer@qt.io>
Date: Mon, 22 Jun 2020 14:26:09 +0200
Subject: [PATCH] Skip proxy widgets that can't take focus when (back)tabbing

Fixes regression introduced in b4981f9d4ca914c6ecaa49bfdd69e51806a3671a,
due to which it was possible to back-tab into a widget even though it or
its focusProxy had a NoFocus policy.

As a drive-by, split the complicated if-statement up a bit for improved
readability.

Change-Id: Ib0ac2604076e812e340b11534c23ae8ae958d082
Fixes: QTBUG-76924
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
(cherry picked from commit 0dbd2dd86389c0705dbe9f518aed12f609ed09a1)
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
---
 src/widgets/kernel/qapplication.cpp           | 10 ++--
 .../widgets/kernel/qwidget/tst_qwidget.cpp    | 47 +++++++++++++++++++
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/src/widgets/kernel/qapplication.cpp b/src/widgets/kernel/qapplication.cpp
index f5644756986..234928793dc 100644
--- a/src/widgets/kernel/qapplication.cpp
+++ b/src/widgets/kernel/qapplication.cpp
@@ -2165,10 +2165,12 @@ QWidget *QApplicationPrivate::focusNextPrevChild_helper(QWidget *toplevel, bool
         // \a next). This is to ensure that we can tab in and out of compound widgets
         // without getting stuck in a tab-loop between parent and child.
         QWidget *focusProxy = test->d_func()->deepestFocusProxy();
-
-        if ((test->focusPolicy() & focus_flag) == focus_flag
-            && !(next && focusProxy && focusProxy->isAncestorOf(test))
-            && !(!next && focusProxy && test->isAncestorOf(focusProxy))
+        const bool canTakeFocus = ((focusProxy ? focusProxy->focusPolicy() : test->focusPolicy())
+                                  & focus_flag) == focus_flag;
+        const bool composites = focusProxy ? (next ? focusProxy->isAncestorOf(test)
+                                                   : test->isAncestorOf(focusProxy))
+                                           : false;
+        if (canTakeFocus && !composites
             && test->isVisibleTo(toplevel) && test->isEnabled()
             && !(w->windowType() == Qt::SubWindow && !w->isAncestorOf(test))
             && (toplevel->windowType() != Qt::SubWindow || toplevel->isAncestorOf(test))) {
diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
index 9d2ed2e14e3..7a39701f3e6 100644
--- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
+++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp
@@ -67,6 +67,7 @@
 #include <qproxystyle.h>
 #include <QtWidgets/QGraphicsView>
 #include <QtWidgets/QGraphicsProxyWidget>
+#include <QtWidgets/QSpinBox>
 #include <QtGui/qwindow.h>
 #include <qtimer.h>
 
@@ -180,6 +181,7 @@ private slots:
     void reverseTabOrder();
     void tabOrderWithProxy();
     void tabOrderWithCompoundWidgets();
+    void tabOrderWithCompoundWidgetsNoFocusPolicy();
     void tabOrderNoChange();
     void tabOrderNoChange2();
 #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT)
@@ -1964,6 +1966,51 @@ static void dumpFocusChain(QWidget *start, bool bForward, const char *desc = nul
 #endif
 }
 
+void tst_QWidget::tabOrderWithCompoundWidgetsNoFocusPolicy()
+{
+    Container container;
+    container.setWindowTitle(QLatin1String(QTest::currentTestFunction()));
+    QSpinBox spinbox1;
+    spinbox1.setObjectName("spinbox1");
+    QSpinBox spinbox2;
+    spinbox2.setObjectName("spinbox2");
+    QSpinBox spinbox3;
+    spinbox3.setObjectName("spinbox3");
+
+    spinbox1.setFocusPolicy(Qt::StrongFocus);
+    spinbox2.setFocusPolicy(Qt::NoFocus);
+    spinbox3.setFocusPolicy(Qt::StrongFocus);
+    container.box->addWidget(&spinbox1);
+    container.box->addWidget(&spinbox2);
+    container.box->addWidget(&spinbox3);
+
+    container.show();
+    container.activateWindow();
+
+    QApplication::setActiveWindow(&container);
+    if (!QTest::qWaitForWindowActive(&container))
+        QSKIP("Window failed to activate, skipping test");
+
+    QVERIFY2(spinbox1.hasFocus(),
+             qPrintable(QApplication::focusWidget()->objectName()));
+    container.tab();
+    QVERIFY2(!spinbox2.hasFocus(),
+             qPrintable(QApplication::focusWidget()->objectName()));
+    QVERIFY2(spinbox3.hasFocus(),
+             qPrintable(QApplication::focusWidget()->objectName()));
+    container.tab();
+    QVERIFY2(spinbox1.hasFocus(),
+             qPrintable(QApplication::focusWidget()->objectName()));
+    container.backTab();
+    QVERIFY2(spinbox3.hasFocus(),
+             qPrintable(QApplication::focusWidget()->objectName()));
+    container.backTab();
+    QVERIFY2(!spinbox2.hasFocus(),
+             qPrintable(QApplication::focusWidget()->objectName()));
+    QVERIFY2(spinbox1.hasFocus(),
+             qPrintable(QApplication::focusWidget()->objectName()));
+}
+
 void tst_QWidget::tabOrderNoChange()
 {
     QWidget w;