From afc2a1c0aa42f6aee0894a7e652832b3cef43729 Mon Sep 17 00:00:00 2001 From: Weng Xuetian Date: Sun, 19 Dec 2021 00:00:22 -0800 Subject: [PATCH] Clean up the focus handling for text input. Usually, a client will only use text input v2/v3. Do not return the focused surface for text input if it has no relevant text input resource. If text-input object is created after surface get the focus, send enter to this text input object. Ensure sendEnter and sendLeave always appear in pair. Also, use the same technique in text-input-v2 for text-input-v3 to handle per resource's enable/disable state, and only send update to enabled text-input-v3 object. --- .../autotests/client/test_text_input_v2.cpp | 45 ++++++++ .../server/test_textinputv3_interface.cpp | 8 ++ src/wayland/seat_interface.cpp | 19 ++- src/wayland/textinput_v2_interface.cpp | 44 ++++--- src/wayland/textinput_v3_interface.cpp | 109 ++++++++++++++---- src/wayland/textinput_v3_interface_p.h | 4 +- 6 files changed, 178 insertions(+), 51 deletions(-) diff --git a/src/wayland/autotests/client/test_text_input_v2.cpp b/src/wayland/autotests/client/test_text_input_v2.cpp index cc30771c9f..c1c07e0f3f 100644 --- a/src/wayland/autotests/client/test_text_input_v2.cpp +++ b/src/wayland/autotests/client/test_text_input_v2.cpp @@ -33,6 +33,7 @@ private Q_SLOTS: void testEnterLeave_data(); void testEnterLeave(); + void testFocusedBeforeCreateTextInput(); void testShowHidePanel(); void testCursorRectangle(); void testPreferredLanguage(); @@ -267,6 +268,50 @@ void TextInputTest::testEnterLeave() QVERIFY(!textInput->enteredSurface()); } +void TextInputTest::testFocusedBeforeCreateTextInput() +{ + // this test verifies that enter leave are sent correctly + QScopedPointer surface(m_compositor->createSurface()); + auto serverSurface = waitForSurface(); + // now let's try to enter it + QSignalSpy textInputChangedSpy(m_seatInterface, &SeatInterface::focusedTextInputSurfaceChanged); + QVERIFY(textInputChangedSpy.isValid()); + QVERIFY(!m_seatInterface->focusedTextInputSurface()); + m_seatInterface->setFocusedKeyboardSurface(serverSurface); + QCOMPARE(m_seatInterface->focusedTextInputSurface(), serverSurface); + QCOMPARE(textInputChangedSpy.count(), 1); + + // This is null because there is no text input object for this client. + QCOMPARE(m_seatInterface->textInputV2()->surface(), nullptr); + + QVERIFY(serverSurface); + QScopedPointer textInput(createTextInput()); + QVERIFY(!textInput.isNull()); + QSignalSpy enteredSpy(textInput.data(), &TextInput::entered); + QVERIFY(enteredSpy.isValid()); + QSignalSpy leftSpy(textInput.data(), &TextInput::left); + QVERIFY(leftSpy.isValid()); + + // and trigger an enter + if (enteredSpy.isEmpty()) { + QVERIFY(enteredSpy.wait()); + } + QCOMPARE(enteredSpy.count(), 1); + QCOMPARE(textInput->enteredSurface(), surface.data()); + + // This is not null anymore because there is a text input object associated with it. + QCOMPARE(m_seatInterface->textInputV2()->surface(), serverSurface); + + // now trigger a leave + m_seatInterface->setFocusedKeyboardSurface(nullptr); + QCOMPARE(textInputChangedSpy.count(), 2); + QVERIFY(leftSpy.wait()); + QVERIFY(!textInput->enteredSurface()); + + QCOMPARE(m_seatInterface->textInputV2()->surface(), nullptr); + QCOMPARE(m_seatInterface->focusedTextInputSurface(), nullptr); +} + void TextInputTest::testShowHidePanel() { // this test verifies that the requests for show/hide panel work diff --git a/src/wayland/autotests/server/test_textinputv3_interface.cpp b/src/wayland/autotests/server/test_textinputv3_interface.cpp index 3e17422916..55acbe8bad 100644 --- a/src/wayland/autotests/server/test_textinputv3_interface.cpp +++ b/src/wayland/autotests/server/test_textinputv3_interface.cpp @@ -553,6 +553,7 @@ void TestTextInputV3Interface::testMultipleTextinputs() m_serverTextInputV3 = m_seat->textInputV3(); QVERIFY(m_serverTextInputV3); + QVERIFY(!m_serverTextInputV3->isEnabled()); QSignalSpy committedSpy(m_serverTextInputV3, &TextInputV3Interface::stateCommitted); // Enable ti1 @@ -560,6 +561,7 @@ void TestTextInputV3Interface::testMultipleTextinputs() ti1->commit(); QVERIFY(committedSpy.wait()); QCOMPARE(committedSpy.last().at(0).value(), 1); + QVERIFY(m_serverTextInputV3->isEnabled()); // Send another three commits on ti1 ti1->enable(); @@ -567,12 +569,14 @@ void TestTextInputV3Interface::testMultipleTextinputs() ti1->commit(); QVERIFY(committedSpy.wait()); QCOMPARE(committedSpy.last().at(0).value(), 2); + QVERIFY(m_serverTextInputV3->isEnabled()); ti1->enable(); ti1->set_content_type(QtWayland::zwp_text_input_v3::content_hint_none, QtWayland::zwp_text_input_v3::content_purpose_normal); ti1->commit(); QVERIFY(committedSpy.wait()); QCOMPARE(committedSpy.last().at(0).value(), 3); + QVERIFY(m_serverTextInputV3->isEnabled()); // at this point total commit count to ti1 is 3 QSignalSpy doneSpy1(ti1, &TextInputV3::done); @@ -590,12 +594,14 @@ void TestTextInputV3Interface::testMultipleTextinputs() ti1->commit(); QVERIFY(committedSpy.wait()); QCOMPARE(committedSpy.last().at(0).value(), 4); + QVERIFY(!m_serverTextInputV3->isEnabled()); // first commit to ti2 ti2->enable(); ti2->commit(); QVERIFY(committedSpy.wait()); QCOMPARE(committedSpy.last().at(0).value(), 1); + QVERIFY(m_serverTextInputV3->isEnabled()); // send commit string m_serverTextInputV3->commitString("Hello world"); @@ -608,6 +614,7 @@ void TestTextInputV3Interface::testMultipleTextinputs() ti2->commit(); QVERIFY(committedSpy.wait()); QCOMPARE(committedSpy.last().at(0).value(), 2); + QVERIFY(!m_serverTextInputV3->isEnabled()); // now re-enable the ti1 and verify sending commits to t2 hasn't affected it's serial // Enable ti1 : 5 commits now @@ -615,6 +622,7 @@ void TestTextInputV3Interface::testMultipleTextinputs() ti1->commit(); QVERIFY(committedSpy.wait()); QCOMPARE(committedSpy.last().at(0).value(), 5); + QVERIFY(m_serverTextInputV3->isEnabled()); // send done signal m_serverTextInputV3->commitString("Hello"); diff --git a/src/wayland/seat_interface.cpp b/src/wayland/seat_interface.cpp index 7051236e2d..a883d963e3 100644 --- a/src/wayland/seat_interface.cpp +++ b/src/wayland/seat_interface.cpp @@ -1184,26 +1184,25 @@ void SeatInterface::setFocusedTextInputSurface(SurfaceInterface *surface) { const quint32 serial = d->display->nextSerial(); + if (d->focusedTextInputSurface == surface) { + return; + } + if (d->focusedTextInputSurface) { disconnect(d->focusedSurfaceDestroyConnection); - } - - if (d->focusedTextInputSurface != surface) { d->textInputV2->d->sendLeave(serial, d->focusedTextInputSurface); d->textInputV3->d->sendLeave(d->focusedTextInputSurface); - d->focusedTextInputSurface = surface; - Q_EMIT focusedTextInputSurfaceChanged(); } + d->focusedTextInputSurface = surface; + Q_EMIT focusedTextInputSurfaceChanged(); - if (d->focusedTextInputSurface) { + if (surface) { d->focusedSurfaceDestroyConnection = connect(surface, &SurfaceInterface::aboutToBeDestroyed, this, [this] { setFocusedTextInputSurface(nullptr); }); + d->textInputV2->d->sendEnter(surface, serial); + d->textInputV3->d->sendEnter(surface); } - - d->textInputV2->d->sendEnter(surface, serial); - d->textInputV3->d->sendEnter(surface); - // TODO: setFocusedSurface like in other interfaces } SurfaceInterface *SeatInterface::focusedTextInputSurface() const diff --git a/src/wayland/textinput_v2_interface.cpp b/src/wayland/textinput_v2_interface.cpp index c5d43f1ae6..385ece3b59 100644 --- a/src/wayland/textinput_v2_interface.cpp +++ b/src/wayland/textinput_v2_interface.cpp @@ -127,7 +127,12 @@ void TextInputManagerV2InterfacePrivate::zwp_text_input_manager_v2_get_text_inpu } TextInputV2InterfacePrivate *textInputPrivate = TextInputV2InterfacePrivate::get(s->textInputV2()); - textInputPrivate->add(resource->client(), id, resource->version()); + auto *textInputResource = textInputPrivate->add(resource->client(), id, resource->version()); + // Send enter to this new text input object if the surface is already focused. + const quint32 serial = s->display()->nextSerial(); + if (textInputPrivate->surface && textInputPrivate->surface->client()->client() == resource->client()) { + textInputPrivate->send_enter(textInputResource->handle, serial, textInputPrivate->surface->resource()); + } } TextInputManagerV2Interface::TextInputManagerV2Interface(Display *display, QObject *parent) @@ -141,25 +146,19 @@ TextInputManagerV2Interface::~TextInputManagerV2Interface() = default; void TextInputV2InterfacePrivate::sendEnter(SurfaceInterface *newSurface, quint32 serial) { EnabledEmitter emitter(q); - if (surface) { - sendLeave(serial, newSurface); - } - + // It should be always synchronized with SeatInterface::focusedTextInputSurface. + Q_ASSERT(!surface && newSurface); surface = newSurface; - if (surface) { - const auto clientResources = textInputsForClient(newSurface->client()); - for (auto resource : clientResources) { - send_enter(resource->handle, serial, newSurface->resource()); - } + const auto clientResources = textInputsForClient(newSurface->client()); + for (auto resource : clientResources) { + send_enter(resource->handle, serial, newSurface->resource()); } } void TextInputV2InterfacePrivate::sendLeave(quint32 serial, SurfaceInterface *leavingSurface) { - if (leavingSurface != surface || !leavingSurface) { - return; - } - + // It should be always synchronized with SeatInterface::focusedTextInputSurface. + Q_ASSERT(leavingSurface && surface == leavingSurface); EnabledEmitter emitter(q); surface.clear(); const auto clientResources = textInputsForClient(leavingSurface->client()); @@ -335,7 +334,14 @@ void TextInputV2InterfacePrivate::zwp_text_input_v2_enable(Resource *resource, w Q_UNUSED(resource) EnabledEmitter emitter(q); auto enabledSurface = SurfaceInterface::get(s); + if (m_enabledSurfaces.contains(enabledSurface)) { + return; + } m_enabledSurfaces.insert(enabledSurface); + QObject::connect(enabledSurface, &SurfaceInterface::aboutToBeDestroyed, q, [this, enabledSurface] { + EnabledEmitter emitter(q); + m_enabledSurfaces.remove(enabledSurface); + }); } void TextInputV2InterfacePrivate::zwp_text_input_v2_disable(Resource *resource, wl_resource *s) @@ -343,8 +349,8 @@ void TextInputV2InterfacePrivate::zwp_text_input_v2_disable(Resource *resource, Q_UNUSED(resource) EnabledEmitter emitter(q); auto disabledSurface = SurfaceInterface::get(s); + QObject::disconnect(disabledSurface, &SurfaceInterface::aboutToBeDestroyed, q, nullptr); m_enabledSurfaces.remove(disabledSurface); - if (disabledSurface == surface) { q->setInputPanelState(false, {0, 0, 0, 0}); } @@ -529,6 +535,14 @@ void TextInputV2Interface::setModifiersMap(const QByteArray &modifiersMap) QPointer TextInputV2Interface::surface() const { + if (!d->surface) { + return nullptr; + } + + if (!d->resourceMap().contains(d->surface->client()->client())) { + return nullptr; + } + return d->surface; } diff --git a/src/wayland/textinput_v3_interface.cpp b/src/wayland/textinput_v3_interface.cpp index d5eb69743b..76dee28d74 100644 --- a/src/wayland/textinput_v3_interface.cpp +++ b/src/wayland/textinput_v3_interface.cpp @@ -108,6 +108,26 @@ TextInputManagerV3InterfacePrivate::TextInputManagerV3InterfacePrivate(TextInput { } +class EnabledEmitter +{ +public: + EnabledEmitter(TextInputV3Interface *q) + : q(q) + , m_wasEnabled(q->isEnabled()) + { + } + ~EnabledEmitter() + { + if (m_wasEnabled != q->isEnabled()) { + Q_EMIT q->enabledChanged(); + } + } + +private: + TextInputV3Interface *q; + const bool m_wasEnabled; +}; + void TextInputManagerV3InterfacePrivate::zwp_text_input_manager_v3_destroy(Resource *resource) { wl_resource_destroy(resource->handle); @@ -121,7 +141,11 @@ void TextInputManagerV3InterfacePrivate::zwp_text_input_manager_v3_get_text_inpu return; } TextInputV3InterfacePrivate *textInputPrivate = TextInputV3InterfacePrivate::get(s->textInputV3()); - textInputPrivate->add(resource->client(), id, resource->version()); + auto *textInputResource = textInputPrivate->add(resource->client(), id, resource->version()); + // Send enter to this new text input object if the surface is already focused. + if (textInputPrivate->surface && textInputPrivate->surface->client()->client() == resource->client()) { + textInputPrivate->send_enter(textInputResource->handle, textInputPrivate->surface->resource()); + } } TextInputManagerV3Interface::TextInputManagerV3Interface(Display *display, QObject *parent) @@ -142,35 +166,37 @@ void TextInputV3InterfacePrivate::zwp_text_input_v3_bind_resource(Resource *reso { // we initialize the serial for the resource to be 0 serialHash.insert(resource, 0); + enabled.insert(resource, false); } void TextInputV3InterfacePrivate::zwp_text_input_v3_destroy(Resource *resource) { // drop resource from the serial hash serialHash.remove(resource); + enabled.remove(resource); } -void TextInputV3InterfacePrivate::sendEnter(SurfaceInterface *s) +void TextInputV3InterfacePrivate::sendEnter(SurfaceInterface *newSurface) { - if (!s) { - return; - } - surface = QPointer(s); - const auto clientResources = textInputsForClient(s->client()); + EnabledEmitter emitter(q); + // It should be always synchronized with SeatInterface::focusedTextInputSurface. + Q_ASSERT(!surface && newSurface); + surface = newSurface; + const auto clientResources = textInputsForClient(newSurface->client()); for (auto resource : clientResources) { - send_enter(resource->handle, s->resource()); + send_enter(resource->handle, newSurface->resource()); } } -void TextInputV3InterfacePrivate::sendLeave(SurfaceInterface *s) +void TextInputV3InterfacePrivate::sendLeave(SurfaceInterface *leavingSurface) { - if (!s) { - return; - } + EnabledEmitter emitter(q); + // It should be always synchronized with SeatInterface::focusedTextInputSurface. + Q_ASSERT(leavingSurface && surface == leavingSurface); surface.clear(); - const auto clientResources = textInputsForClient(s->client()); + const auto clientResources = textInputsForClient(leavingSurface->client()); for (auto resource : clientResources) { - send_leave(resource->handle, s->resource()); + send_leave(resource->handle, leavingSurface->resource()); } } @@ -179,7 +205,7 @@ void TextInputV3InterfacePrivate::sendPreEdit(const QString &text, const quint32 if (!surface) { return; } - const QList textInputs = textInputsForClient(surface->client()); + const QList textInputs = enabledTextInputsForClient(surface->client()); for (auto resource : textInputs) { send_preedit_string(resource->handle, text, cursorBegin, cursorEnd); } @@ -190,7 +216,7 @@ void TextInputV3InterfacePrivate::commitString(const QString &text) if (!surface) { return; } - const QList textInputs = textInputsForClient(surface->client()); + const QList textInputs = enabledTextInputsForClient(surface->client()); for (auto resource : textInputs) { send_commit_string(resource->handle, text); } @@ -201,7 +227,7 @@ void TextInputV3InterfacePrivate::deleteSurroundingText(quint32 before, quint32 if (!surface) { return; } - const QList textInputs = textInputsForClient(surface->client()); + const QList textInputs = enabledTextInputsForClient(surface->client()); for (auto resource : textInputs) { send_delete_surrounding_text(resource->handle, before, after); } @@ -212,7 +238,8 @@ void TextInputV3InterfacePrivate::done() if (!surface) { return; } - const QList textInputs = textInputsForClient(surface->client()); + const QList textInputs = enabledTextInputsForClient(surface->client()); + for (auto resource : textInputs) { // zwp_text_input_v3.done takes the serial argument which is equal to number of commit requests issued send_done(resource->handle, serialHash[resource]); @@ -224,6 +251,29 @@ QList TextInputV3InterfacePrivate::text return resourceMap().values(client->client()); } +QList TextInputV3InterfacePrivate::enabledTextInputsForClient(ClientConnection *client) const +{ + QList result; + const auto [start, end] = resourceMap().equal_range(client->client()); + for (auto it = start; it != end; ++it) { + if (enabled[*it]) { + result.append(*it); + } + } + return result; +} + +bool TextInputV3InterfacePrivate::isEnabled() const +{ + if (!surface) { + return false; + } + const auto clientResources = textInputsForClient(surface->client()); + return std::any_of(clientResources.begin(), clientResources.end(), [this](Resource *resource) { + return enabled[resource]; + }); +} + void TextInputV3InterfacePrivate::zwp_text_input_v3_enable(Resource *resource) { // reset pending state to default @@ -284,11 +334,12 @@ void TextInputV3InterfacePrivate::zwp_text_input_v3_set_text_change_cause(Resour void TextInputV3InterfacePrivate::zwp_text_input_v3_commit(Resource *resource) { + EnabledEmitter emitter(q); serialHash[resource]++; - if (enabled != pending.enabled) { - enabled = pending.enabled; - Q_EMIT q->enabledChanged(); + auto &resourceEnabled = enabled[resource]; + if (resourceEnabled != pending.enabled) { + resourceEnabled = pending.enabled; } if (surroundingTextChangeCause != pending.surroundingTextChangeCause) { @@ -299,14 +350,14 @@ void TextInputV3InterfacePrivate::zwp_text_input_v3_commit(Resource *resource) if (contentHints != pending.contentHints || contentPurpose != pending.contentPurpose) { contentHints = pending.contentHints; contentPurpose = pending.contentPurpose; - if (enabled) { + if (resourceEnabled) { Q_EMIT q->contentTypeChanged(); } } if (cursorRectangle != pending.cursorRectangle) { cursorRectangle = pending.cursorRectangle; - if (enabled) { + if (resourceEnabled) { Q_EMIT q->cursorRectangleChanged(cursorRectangle); } } @@ -316,7 +367,7 @@ void TextInputV3InterfacePrivate::zwp_text_input_v3_commit(Resource *resource) surroundingText = pending.surroundingText; surroundingTextCursorPosition = pending.surroundingTextCursorPosition; surroundingTextSelectionAnchor = pending.surroundingTextSelectionAnchor; - if (enabled) { + if (resourceEnabled) { Q_EMIT q->surroundingTextChanged(); } } @@ -391,6 +442,14 @@ void TextInputV3Interface::done() QPointer TextInputV3Interface::surface() const { + if (!d->surface) { + return nullptr; + } + + if (!d->resourceMap().contains(d->surface->client()->client())) { + return nullptr; + } + return d->surface; } @@ -401,7 +460,7 @@ QRect TextInputV3Interface::cursorRectangle() const bool TextInputV3Interface::isEnabled() const { - return d->enabled; + return d->isEnabled(); } } diff --git a/src/wayland/textinput_v3_interface_p.h b/src/wayland/textinput_v3_interface_p.h index 017b27ab85..e3fa011d50 100644 --- a/src/wayland/textinput_v3_interface_p.h +++ b/src/wayland/textinput_v3_interface_p.h @@ -42,7 +42,9 @@ public: void deleteSurroundingText(quint32 beforeLength, quint32 afterLength); void done(); + bool isEnabled() const; QList textInputsForClient(ClientConnection *client) const; + QList enabledTextInputsForClient(ClientConnection *client) const; static TextInputV3InterfacePrivate *get(TextInputV3Interface *inputInterface) { @@ -55,7 +57,6 @@ public: SeatInterface *seat = nullptr; QPointer surface; - bool enabled = false; QString surroundingText; qint32 surroundingTextCursorPosition = 0; @@ -74,6 +75,7 @@ public: } pending; QHash serialHash; + QHash enabled; void defaultPending();