diff options
author | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-11-29 20:30:53 +0100 |
---|---|---|
committer | Volker Hilsheimer <volker.hilsheimer@qt.io> | 2021-12-04 22:13:57 +0100 |
commit | 42d411e2e80c434f3e62cc55e32da2eeaa6149ae (patch) | |
tree | 1e0689ba7091b3c32cd16fb0eeed39ee5f8a4612 | |
parent | 61ee61d8c977ecf876ce1d364cff634467b562a9 (diff) |
Set PopupFocusReason when focus moves because a popup opens or closes
When a popup opens and takes keyboard focus, then the control that had
focus needs to get a focusOut event, with the reason set correctly. This
allows text controls to stop blinking, but not deselecting text. So set
the reason parameter to Qt::PopupFocusReason in all calls to functions
that change focus in response to a QQuickPopup showing or hiding.
However, QQuickItem partially ignored focus changes where the reason was
set to Qt::PopupFocusReason. This seems to be a left-over from a fix for
issues with Qt Quick Control 1 made in c860d33437c59a35d9d17ad199ce4f0f
to make sure that menu entries are enabled and selection is not removed.
The reason given here is no longer relevant: of course popups need to
take focus, otherwise we can't navigate menus with the keyboard; and
they do, at the latest the MenuItem takes focus away from the control
breaking the entire assumption. And since prior to this change no code
other than the outdated tst_qquicktextinput test passed focus with
Qt::PopupFocusReason, we can safely assume that this logic is no longer
needed. Controls that want to ignore a focusOut with PopupFocusReason
can do so with this change.
Note: PopupFocusReason in Qt Quick means any popup, including dialogs,
while in QtWidgets it means a Qt::Popup type window. Since Quick dialogs
don't trigger window activation changes and focusOut events with
corresponding focus reasons, and since QtQuick has no concept equivalent
to Qt::Popup where this could become confusing, this seems acceptable.
Fixes: QTBUG-71723
Task-number: QTBUG-75862
Task-number: QTBUG-36332
Pick-to: 6.2
Change-Id: I7a0a29830d8f7ed80b22411785214758b896562c
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
-rw-r--r-- | src/quick/items/qquickitem.cpp | 14 | ||||
-rw-r--r-- | src/quicktemplates2/qquickpopup.cpp | 10 | ||||
-rw-r--r-- | tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp | 2 | ||||
-rw-r--r-- | tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp | 2 | ||||
-rw-r--r-- | tests/auto/quickcontrols2/focus/tst_focus.cpp | 1 |
5 files changed, 12 insertions, 17 deletions
diff --git a/src/quick/items/qquickitem.cpp b/src/quick/items/qquickitem.cpp index 4ed587fe38..4257ff30cc 100644 --- a/src/quick/items/qquickitem.cpp +++ b/src/quick/items/qquickitem.cpp @@ -7595,14 +7595,12 @@ void QQuickItem::setFocus(bool focus, Qt::FocusReason reason) while (scope && !scope->isFocusScope() && scope->parentItem()) scope = scope->parentItem(); if (d->window) { - if (reason != Qt::PopupFocusReason) { - auto da = d->deliveryAgentPrivate(); - Q_ASSERT(da); - if (focus) - da->setFocusInScope(scope, this, reason); - else - da->clearFocusInScope(scope, this, reason); - } + auto da = d->deliveryAgentPrivate(); + Q_ASSERT(da); + if (focus) + da->setFocusInScope(scope, this, reason); + else + da->clearFocusInScope(scope, this, reason); } else { // do the focus changes from setFocusInScope/clearFocusInScope that are // unrelated to a window diff --git a/src/quicktemplates2/qquickpopup.cpp b/src/quicktemplates2/qquickpopup.cpp index 569c923433..4ca4dd9272 100644 --- a/src/quicktemplates2/qquickpopup.cpp +++ b/src/quicktemplates2/qquickpopup.cpp @@ -476,7 +476,7 @@ bool QQuickPopupPrivate::prepareEnterTransition() emit q->visibleChanged(); if (focus) - popupItem->setFocus(true); + popupItem->setFocus(true, Qt::PopupFocusReason); } return true; } @@ -498,7 +498,7 @@ bool QQuickPopupPrivate::prepareExitTransition() if (!hadActiveFocusBeforeExitTransition) hadActiveFocusBeforeExitTransition = popupItem->hasActiveFocus(); if (focus) - popupItem->setFocus(false); + popupItem->setFocus(false, Qt::PopupFocusReason); transitionState = ExitTransition; hideOverlay(); emit q->aboutToHide(); @@ -540,13 +540,13 @@ void QQuickPopupPrivate::finalizeExitTransition() } } if (nextFocusPopup) { - nextFocusPopup->forceActiveFocus(); + nextFocusPopup->forceActiveFocus(Qt::PopupFocusReason); } else { QQuickApplicationWindow *applicationWindow = qobject_cast<QQuickApplicationWindow*>(window); if (applicationWindow) - applicationWindow->contentItem()->setFocus(true); + applicationWindow->contentItem()->setFocus(true, Qt::PopupFocusReason); else - window->contentItem()->setFocus(true); + window->contentItem()->setFocus(true, Qt::PopupFocusReason); } } diff --git a/tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp b/tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp index 41ff01544a..812b541cb0 100644 --- a/tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp +++ b/tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp @@ -1368,7 +1368,7 @@ void tst_qquicktextedit::selectionOnFocusOut() QVERIFY(edit2->hasActiveFocus()); edit2->setFocus(false, Qt::PopupFocusReason); - QVERIFY(edit2->hasActiveFocus()); + QVERIFY(!edit2->hasActiveFocus()); QCOMPARE(edit2->selectedText(), QLatin1String("text 2")); } diff --git a/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp b/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp index 5e47476869..669f94ed48 100644 --- a/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp +++ b/tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp @@ -3756,8 +3756,6 @@ void tst_qquicktextinput::focusOutNotClearSelection() input.setFocus(false, Qt::PopupFocusReason); QGuiApplication::processEvents(); QTRY_COMPARE(input.selectedText(), QLatin1String("llo")); - // QTBUG-36332 and 36292: a popup window does not take focus - QTRY_COMPARE(input.hasActiveFocus(), true); input.setFocus(true); QTRY_COMPARE(input.hasActiveFocus(), true); diff --git a/tests/auto/quickcontrols2/focus/tst_focus.cpp b/tests/auto/quickcontrols2/focus/tst_focus.cpp index 6f1bacee6a..dfaded4056 100644 --- a/tests/auto/quickcontrols2/focus/tst_focus.cpp +++ b/tests/auto/quickcontrols2/focus/tst_focus.cpp @@ -432,7 +432,6 @@ void tst_focus::reason() // Popup opens -> PopupFocusReason QTest::mouseClick(&view, Qt::RightButton, {}, itemCenter(control)); QTRY_VERIFY(!customItem->hasActiveFocus()); - QEXPECT_FAIL("", "Popup opening doesn't set the focus reason", Continue); QCOMPARE(customItem->focusReason(), Qt::PopupFocusReason); QTest::keyClick(&view, Qt::Key_Escape); // close the popup } |