aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVolker Hilsheimer <volker.hilsheimer@qt.io>2021-11-29 20:30:53 +0100
committerVolker Hilsheimer <volker.hilsheimer@qt.io>2021-12-04 22:13:57 +0100
commit42d411e2e80c434f3e62cc55e32da2eeaa6149ae (patch)
tree1e0689ba7091b3c32cd16fb0eeed39ee5f8a4612
parent61ee61d8c977ecf876ce1d364cff634467b562a9 (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.cpp14
-rw-r--r--src/quicktemplates2/qquickpopup.cpp10
-rw-r--r--tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp2
-rw-r--r--tests/auto/quick/qquicktextinput/tst_qquicktextinput.cpp2
-rw-r--r--tests/auto/quickcontrols2/focus/tst_focus.cpp1
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
}