From d887758bf4727d891ae05958d2c10d395c34be81 Mon Sep 17 00:00:00 2001 From: Doris Verria Date: Wed, 5 Jun 2024 10:05:42 +0200 Subject: [PATCH] Make QQuickItemPrivate::setLastFocusChangeReason virtual Fix a regression that 644ee4d23464cb04b5162051ffa1524f006b544d introduced by moving focusReason to QQuickItemPrivate. The already existing QQuickControl::focusReasonChanged signal was not being emitted anymore when the focus would change. To fix, make setLastFocusChangeReason virtual in QQuickItemPrivate so that the subclasses can override and emit the relevant signals. Fixes: QTBUG-125725 Pick-to: 6.7 6.8 Change-Id: I88d664ec4fa3cc9262ee2703d63e7e716cac68e9 Reviewed-by: Volker Hilsheimer --- src/quick/items/qquickitem.cpp | 5 ++-- src/quick/items/qquickitem_p.h | 2 +- src/quicktemplates/qquickcontrol.cpp | 29 +++++++++--------- src/quicktemplates/qquickcontrol_p_p.h | 2 ++ src/quicktemplates/qquicktextarea.cpp | 11 ++++++- src/quicktemplates/qquicktextarea_p_p.h | 2 ++ src/quicktemplates/qquicktextfield.cpp | 11 ++++++- src/quicktemplates/qquicktextfield_p_p.h | 2 ++ tests/auto/quickcontrols/focus/tst_focus.cpp | 31 +++++++++++++++++++- 9 files changed, 74 insertions(+), 21 deletions(-) diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp index e7b2a31f04..84efbd891e 100644 --- a/src/quick/items/qquickitem.cpp +++ b/src/quick/items/qquickitem.cpp @@ -1744,12 +1744,13 @@ Qt::FocusReason QQuickItemPrivate::lastFocusChangeReason() const return static_cast(focusReason); } -void QQuickItemPrivate::setLastFocusChangeReason(Qt::FocusReason reason) +bool QQuickItemPrivate::setLastFocusChangeReason(Qt::FocusReason reason) { if (focusReason == reason) - return; + return false; focusReason = reason; + return true; } /*! diff --git a/src/quick/items/qquickitem_p.h b/src/quick/items/qquickitem_p.h index bb238904ca..ab3a2a7cfb 100644 --- a/src/quick/items/qquickitem_p.h +++ b/src/quick/items/qquickitem_p.h @@ -564,7 +564,7 @@ public: bool setFocusIfNeeded(QEvent::Type); Qt::FocusReason lastFocusChangeReason() const; - void setLastFocusChangeReason(Qt::FocusReason reason); + virtual bool setLastFocusChangeReason(Qt::FocusReason reason); QTransform windowToItemTransform() const; QTransform itemToWindowTransform() const; diff --git a/src/quicktemplates/qquickcontrol.cpp b/src/quicktemplates/qquickcontrol.cpp index 72d298d0af..667d778180 100644 --- a/src/quicktemplates/qquickcontrol.cpp +++ b/src/quicktemplates/qquickcontrol.cpp @@ -911,6 +911,19 @@ void QQuickControlPrivate::itemFocusChanged(QQuickItem *item, Qt::FocusReason re setLastFocusChangeReason(reason); } +bool QQuickControlPrivate::setLastFocusChangeReason(Qt::FocusReason reason) +{ + Q_Q(QQuickControl); + Qt::FocusReason oldReason = static_cast(focusReason); + const auto focusReasonChanged = QQuickItemPrivate::setLastFocusChangeReason(reason); + if (focusReasonChanged) + emit q->focusReasonChanged(); + if (isKeyFocusReason(oldReason) != isKeyFocusReason(reason)) + emit q->visualFocusChanged(); + + return focusReasonChanged; +} + QQuickControl::QQuickControl(QQuickItem *parent) : QQuickItem(*(new QQuickControlPrivate), parent) { @@ -1384,11 +1397,7 @@ Qt::FocusReason QQuickControl::focusReason() const void QQuickControl::setFocusReason(Qt::FocusReason reason) { Q_D(QQuickControl); - Qt::FocusReason oldReason = static_cast(d->focusReason); d->setLastFocusChangeReason(reason); - emit focusReasonChanged(); - if (isKeyFocusReason(oldReason) != isKeyFocusReason(reason)) - emit visualFocusChanged(); } /*! @@ -1403,7 +1412,7 @@ void QQuickControl::setFocusReason(Qt::FocusReason reason) \l Item::activeFocus. This ensures that key focus is only visualized when interacting with keys - not when interacting via touch or mouse. - \sa Item::focusReason, Item::activeFocus + \sa focusReason, Item::activeFocus */ bool QQuickControl::hasVisualFocus() const { @@ -1969,22 +1978,12 @@ QFont QQuickControl::defaultFont() const void QQuickControl::focusInEvent(QFocusEvent *event) { - Q_D(QQuickControl); - Qt::FocusReason oldReason = static_cast(d->focusReason); QQuickItem::focusInEvent(event); - Qt::FocusReason reason = event->reason(); - if (isKeyFocusReason(oldReason) != isKeyFocusReason(reason)) - emit visualFocusChanged(); } void QQuickControl::focusOutEvent(QFocusEvent *event) { - Q_D(QQuickControl); - Qt::FocusReason oldReason = static_cast(d->focusReason); QQuickItem::focusOutEvent(event); - Qt::FocusReason reason = event->reason(); - if (isKeyFocusReason(oldReason) != isKeyFocusReason(reason)) - emit visualFocusChanged(); } #if QT_CONFIG(quicktemplates2_hover) diff --git a/src/quicktemplates/qquickcontrol_p_p.h b/src/quicktemplates/qquickcontrol_p_p.h index c58ee33ede..24d0507670 100644 --- a/src/quicktemplates/qquickcontrol_p_p.h +++ b/src/quicktemplates/qquickcontrol_p_p.h @@ -155,6 +155,8 @@ public: void itemDestroyed(QQuickItem *item) override; void itemFocusChanged(QQuickItem *item, Qt::FocusReason reason) override; + bool setLastFocusChangeReason(Qt::FocusReason) override; + virtual qreal getContentWidth() const; virtual qreal getContentHeight() const; diff --git a/src/quicktemplates/qquicktextarea.cpp b/src/quicktemplates/qquicktextarea.cpp index fb4702928e..a53f4ac542 100644 --- a/src/quicktemplates/qquicktextarea.cpp +++ b/src/quicktemplates/qquicktextarea.cpp @@ -472,6 +472,16 @@ QPalette QQuickTextAreaPrivate::defaultPalette() const return QQuickTheme::palette(QQuickTheme::TextArea); } +bool QQuickTextAreaPrivate::setLastFocusChangeReason(Qt::FocusReason reason) +{ + Q_Q(QQuickTextArea); + const auto focusReasonChanged = QQuickItemPrivate::setLastFocusChangeReason(reason); + if (focusReasonChanged) + emit q->focusReasonChanged(); + + return focusReasonChanged; +} + QQuickTextArea::QQuickTextArea(QQuickItem *parent) : QQuickTextEdit(*(new QQuickTextAreaPrivate), parent) { @@ -663,7 +673,6 @@ void QQuickTextArea::setFocusReason(Qt::FocusReason reason) { Q_D(QQuickTextArea); d->setLastFocusChangeReason(reason); - emit focusReasonChanged(); } diff --git a/src/quicktemplates/qquicktextarea_p_p.h b/src/quicktemplates/qquicktextarea_p_p.h index 5e406d869e..849c22b97c 100644 --- a/src/quicktemplates/qquicktextarea_p_p.h +++ b/src/quicktemplates/qquicktextarea_p_p.h @@ -96,6 +96,8 @@ public: QPalette defaultPalette() const override; + bool setLastFocusChangeReason(Qt::FocusReason reason) override; + #if QT_CONFIG(quicktemplates2_hover) bool hovered = false; bool explicitHoverEnabled = false; diff --git a/src/quicktemplates/qquicktextfield.cpp b/src/quicktemplates/qquicktextfield.cpp index 9513be3f3b..7163f2a302 100644 --- a/src/quicktemplates/qquicktextfield.cpp +++ b/src/quicktemplates/qquicktextfield.cpp @@ -364,6 +364,16 @@ QPalette QQuickTextFieldPrivate::defaultPalette() const return QQuickTheme::palette(QQuickTheme::TextField); } +bool QQuickTextFieldPrivate::setLastFocusChangeReason(Qt::FocusReason reason) +{ + Q_Q(QQuickTextField); + const auto focusReasonChanged = QQuickItemPrivate::setLastFocusChangeReason(reason); + if (focusReasonChanged) + emit q->focusReasonChanged(); + + return focusReasonChanged; +} + QQuickTextField::QQuickTextField(QQuickItem *parent) : QQuickTextInput(*(new QQuickTextFieldPrivate), parent) { @@ -549,7 +559,6 @@ void QQuickTextField::setFocusReason(Qt::FocusReason reason) { Q_D(QQuickTextField); d->setLastFocusChangeReason(reason); - emit focusReasonChanged(); } /*! diff --git a/src/quicktemplates/qquicktextfield_p_p.h b/src/quicktemplates/qquicktextfield_p_p.h index c8afee8ba3..ac75460b7e 100644 --- a/src/quicktemplates/qquicktextfield_p_p.h +++ b/src/quicktemplates/qquicktextfield_p_p.h @@ -94,6 +94,8 @@ public: QPalette defaultPalette() const override; + bool setLastFocusChangeReason(Qt::FocusReason reason) override; + #if QT_CONFIG(quicktemplates2_hover) bool hovered = false; bool explicitHoverEnabled = false; diff --git a/tests/auto/quickcontrols/focus/tst_focus.cpp b/tests/auto/quickcontrols/focus/tst_focus.cpp index dde4621060..9f68e3b61d 100644 --- a/tests/auto/quickcontrols/focus/tst_focus.cpp +++ b/tests/auto/quickcontrols/focus/tst_focus.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -244,7 +245,7 @@ void tst_focus::reason() QQuickControl *customText = view.findChild("customText"); QQuickControl *customItem = view.findChild("customItem"); // not a QQuickControl subclass - QQuickItem *textfield = view.findChild("textfield"); + QQuickTextField *textfield = view.findChild("textfield"); // helper for clicking into a control const auto itemCenter = [](const QQuickItem *item) -> QPoint { @@ -273,21 +274,35 @@ void tst_focus::reason() } QCOMPARE(control->focusReason(), Qt::ActiveWindowFocusReason); + QSignalSpy controlSpy(control, SIGNAL(focusReasonChanged())); + QSignalSpy textfieldSpy(textfield, SIGNAL(focusReasonChanged())); + QSignalSpy customItemSpy(customItem, SIGNAL(focusReasonChanged())); + int controlSpyCount = 0, textfieldSpyCount = 0, customItemSpyCount = 0; + QVERIFY(!controlSpy.count()); + QVERIFY(!textfieldSpy.count()); + QVERIFY(!customItemSpy.count()); + // test setter/getter control->setFocus(false, Qt::MouseFocusReason); QCOMPARE(control->focusReason(), Qt::MouseFocusReason); + QCOMPARE(controlSpy.count(), ++controlSpyCount); control->setFocus(true, Qt::TabFocusReason); QCOMPARE(control->focusReason(), Qt::TabFocusReason); + QCOMPARE(controlSpy.count(), ++controlSpyCount); control->setFocus(false, Qt::BacktabFocusReason); QCOMPARE(control->focusReason(), Qt::BacktabFocusReason); + QCOMPARE(controlSpy.count(), ++controlSpyCount); control->forceActiveFocus(Qt::ShortcutFocusReason); QCOMPARE(control->focusReason(), Qt::ShortcutFocusReason); + QCOMPARE(controlSpy.count(), ++controlSpyCount); control->setFocusReason(Qt::PopupFocusReason); QCOMPARE(control->focusReason(), Qt::PopupFocusReason); + QCOMPARE(controlSpy.count(), ++controlSpyCount); // programmatic focus changes combobox->setFocus(true, Qt::OtherFocusReason); QCOMPARE(control->focusReason(), Qt::OtherFocusReason); + QCOMPARE(controlSpy.count(), ++controlSpyCount); QVERIFY(combobox->hasFocus()); QVERIFY(combobox->hasActiveFocus()); @@ -324,27 +339,35 @@ void tst_focus::reason() QVERIFY(customItem->hasActiveFocus()); QCOMPARE(customText->focusReason(), Qt::TabFocusReason); QCOMPARE(customItem->focusReason(), Qt::TabFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); customItem->setFocusReason(Qt::NoFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); QTest::keyClick(&view, Qt::Key_Tab); QVERIFY(textfield->hasFocus()); QVERIFY(textfield->hasActiveFocus()); QCOMPARE(qApp->focusObject(), textfield); QCOMPARE(customItem->focusReason(), Qt::TabFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); + QCOMPARE(textfieldSpy.count(), ++textfieldSpyCount); QTest::keyClick(&view, Qt::Key_Tab); QVERIFY(control->hasFocus()); QVERIFY(control->hasActiveFocus()); QCOMPARE(control->focusReason(), Qt::TabFocusReason); + QCOMPARE(controlSpy.count(), ++controlSpyCount); // backtab -> BacktabFocusReason QTest::keyClick(&view, Qt::Key_Tab, Qt::ShiftModifier); QVERIFY(textfield->hasFocus()); QCOMPARE(control->focusReason(), Qt::BacktabFocusReason); + QCOMPARE(controlSpy.count(), ++controlSpyCount); + QCOMPARE(textfieldSpy.count(), ++textfieldSpyCount); QTest::keyClick(&view, Qt::Key_Tab, Qt::ShiftModifier); QVERIFY(customItem->hasFocus()); QCOMPARE(customItem->focusReason(), Qt::BacktabFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); QTest::keyClick(&view, Qt::Key_Tab, Qt::ShiftModifier); QVERIFY(customText->hasFocus()); @@ -399,11 +422,15 @@ void tst_focus::reason() QVERIFY(customItem->hasActiveFocus()); QCOMPARE(customText->focusReason(), Qt::MouseFocusReason); QCOMPARE(customItem->focusReason(), Qt::MouseFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); customItem->setFocusReason(Qt::NoFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); QTest::mouseClick(&view, Qt::LeftButton, {}, itemCenter(textfield)); QCOMPARE(customItem->focusReason(), Qt::MouseFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); customItem->setFocusReason(Qt::NoFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); customText->setFocusReason(Qt::NoFocusReason); #if QT_CONFIG(wheelevent) @@ -414,11 +441,13 @@ void tst_focus::reason() QGuiApplication::sendEvent(customItem, &wheelEvent); QVERIFY(customItem->hasActiveFocus()); QCOMPARE(customItem->focusReason(), Qt::MouseFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); // Popup opens -> PopupFocusReason QTest::mouseClick(&view, Qt::RightButton, {}, itemCenter(control)); QTRY_VERIFY(!customItem->hasActiveFocus()); QCOMPARE(customItem->focusReason(), Qt::PopupFocusReason); + QCOMPARE(customItemSpy.count(),++customItemSpyCount); QTest::keyClick(&view, Qt::Key_Escape); // close the popup #endif }