Don't move focus away from previous proxy in QWidget::setFocusProxy

This amends 23b998f, and the commits
3e74634 and
9478831. This change restores the
pre-5.13.1 behavior of setFocusProxy to not move focus away from a
previously set focus proxy.

With the previous changes, focus would move away from a proxy when a
new proxy is set, if the old proxy had focus. While there are arguments
in favor of this behavior, it is a change of behavior that shouldn't
be introduced to 20+ years old functionality in order to fix the real
bugs addressed by the initial commits.

Instead, move focus only to the new proxy when the focus widget was
the widget that gets a focus proxy.

[ChangeLog][QtWidgets][QWidget] setFocusProxy no longer moves focus
away from a previously set focus proxy, restoring pre-Qt 5.13.1
behavior.

Change-Id: Icf2ad7cba5b860014aeef91cb274c442a2ab9d42
Fixes: QTBUG-83720
Reviewed-by: David Faure <[email protected]>
(cherry picked from commit a8fee0b)

src/widgets/kernel/qwidget.cpp

@@ -6178,7 +6178,8 @@ void QWidget::setWindowRole(const QString &role)
 
     setFocusProxy() sets the widget which will actually get focus when
     "this widget" gets it. If there is a focus proxy, setFocus() and
-    hasFocus() operate on the focus proxy.
+    hasFocus() operate on the focus proxy. If "this widget" is the focus
+    widget, then setFocusProxy() moves focus to the new focus proxy.
 
     \sa focusProxy()
 */
@@ -6196,16 +6197,12 @@ void QWidget::setFocusProxy(QWidget * w)
         }
     }
 
-    QWidget *oldDeepestFocusProxy = d->deepestFocusProxy();
-    if (!oldDeepestFocusProxy)
-        oldDeepestFocusProxy = this;
-
-    const bool focusProxyHadFocus = (QApplicationPrivate::focus_widget == oldDeepestFocusProxy);
+    const bool moveFocusToProxy = (QApplicationPrivate::focus_widget == this);
 
     d->createExtra();
     d->extra->focus_proxy = w;
 
-    if (focusProxyHadFocus)
+    if (moveFocusToProxy)
         setFocus(Qt::OtherFocusReason);
 }
 

tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp

@@ -2235,12 +2235,16 @@ void tst_QWidget::appFocusWidgetWhenLosingFocusProxy()
     QApplication::setActiveWindow(&window);
     QVERIFY(QTest::qWaitForWindowActive(&window));
     QCOMPARE(QApplication::focusWidget(), lineEditFocusProxy);
+    QVERIFY(lineEdit->hasFocus());
+    QVERIFY(lineEditFocusProxy->hasFocus());
 
     // When unsetting the focus proxy
     lineEdit->setFocusProxy(nullptr);
 
-    // Then the application focus widget should be back to the lineedit
-    QCOMPARE(QApplication::focusWidget(), lineEdit);
+    // then the focus widget should not change
+    QCOMPARE(QApplication::focusWidget(), lineEditFocusProxy);
+    QVERIFY(!lineEdit->hasFocus());
+    QVERIFY(lineEditFocusProxy->hasFocus());
 }
 
 void tst_QWidget::explicitTabOrderWithComplexWidget()
@@ -10089,6 +10093,7 @@ void tst_QWidget::openModal_taskQTBUG_5804()
 void tst_QWidget::focusProxy()
 {
     QWidget window;
+    window.setFocusPolicy(Qt::StrongFocus);
     class Container : public QWidget
     {
     public:
@@ -10136,39 +10141,63 @@ void tst_QWidget::focusProxy()
     layout->addWidget(container2);
     window.setLayout(layout);
 
+    window.setFocus();
     window.show();
     window.activateWindow();
     if (!QTest::qWaitForWindowExposed(&window) || !QTest::qWaitForWindowActive(&window))
         QSKIP("Window activation failed");
 
-    QCOMPARE(container1->focusInCount, 1);
+    // given a widget without focus proxy
+    QVERIFY(window.hasFocus());
+    QCOMPARE(&window, QApplication::focusWidget());
+    QVERIFY(!container1->hasFocus());
+    QVERIFY(!container2->hasFocus());
+    QCOMPARE(container1->focusInCount, 0);
     QCOMPARE(container1->focusOutCount, 0);
 
-    // given a widget with a nested focus proxy
+    // setting a (nested) focus proxy moves focus
     window.setFocusProxy(container1);
     QCOMPARE(window.focusWidget(), container1->edit);
     QCOMPARE(window.focusWidget(), QApplication::focusWidget());
+    QVERIFY(window.hasFocus()); // and redirects hasFocus correctly
     QVERIFY(container1->edit->hasFocus());
     QCOMPARE(container1->focusInCount, 1);
 
-    // changing the focus proxy should move focus to the new proxy
+    // changing the focus proxy should not move focus
     window.setFocusProxy(container2);
+    QCOMPARE(window.focusWidget(), container1->edit);
+    QCOMPARE(window.focusWidget(), QApplication::focusWidget());
+    QVERIFY(!window.hasFocus());
+    QCOMPARE(container1->focusOutCount, 0);
+
+    // but setting focus again does
+    window.setFocus();
     QCOMPARE(window.focusWidget(), container2->edit);
     QCOMPARE(window.focusWidget(), QApplication::focusWidget());
+    QVERIFY(window.hasFocus());
     QVERIFY(!container1->edit->hasFocus());
     QVERIFY(container2->edit->hasFocus());
     QCOMPARE(container1->focusInCount, 1);
     QCOMPARE(container1->focusOutCount, 1);
     QCOMPARE(container2->focusInCount, 1);
     QCOMPARE(container2->focusOutCount, 0);
 
-    // clearing the focus proxy moves focus
+    // clearing the focus proxy does not move focus
     window.setFocusProxy(nullptr);
-    QCOMPARE(window.focusWidget(), &window);
+    QCOMPARE(window.focusWidget(), container2->edit);
     QCOMPARE(window.focusWidget(), QApplication::focusWidget());
+    QVERIFY(!window.hasFocus());
     QCOMPARE(container1->focusInCount, 1);
     QCOMPARE(container1->focusOutCount, 1);
     QCOMPARE(container2->focusInCount, 1);
+    QCOMPARE(container2->focusOutCount, 0);
+
+    // but clearing focus does
+    window.focusWidget()->clearFocus();
+    QCOMPARE(QApplication::focusWidget(), nullptr);
+    QVERIFY(!window.hasFocus());
+    QVERIFY(!container2->hasFocus());
+    QVERIFY(!container2->edit->hasFocus());
     QCOMPARE(container2->focusOutCount, 1);
 }