wayland: input method protocol #195

CMurtagh-LGTM · 2025-08-22T17:27:35Z

~~Popup surface~~ This PR is already large, will add in future PR.
Handle surrounding_text, text_change_cause and content_type events somehow.
Key repeat
Documentation

~~Do we want to also expose the virtual keyboard so people can create their own clients?~~

Popup surface and qml virtual keyboard will be in future PRs.

outfoxxed · 2025-09-03T06:02:07Z

~~Exposing virtual keyboard would be a good idea, but as a separate PR.~~
Oh I see it was added. Is this ready for review?

CMurtagh-LGTM · 2025-09-03T06:50:38Z

I still need to do a couple things - especially documentation. I'll add the popup screen and exposing virtual keyboard in future prs since this one is already quite large.

CMurtagh-LGTM · 2025-11-17T03:41:39Z

Sorry I haven't updated in a while.
This pull request is good to go.

I have a separate branch that I have been working on the adding the popup surface functionality however it looks like I need to use the private qt headers, which I have not yet been able to work out.

outfoxxed · 2025-11-18T05:04:47Z

Looks like CI is broken here, can you rebase on master? For private qt headers you should just be able to import them if you add a dependency on the private qt module.

outfoxxed

Reviewed most of it that didn't seem likely to greatly change. It would also be useful if you included a couple tests. They dont have to be complex or automatic, just some QML files we can run to make sure its still working.

outfoxxed · 2025-11-21T06:58:56Z

src/wayland/input_method/input-method-unstable-v2.xml

Why is this protocol included directly instead of from w-p?

outfoxxed · 2025-11-21T07:16:05Z

src/wayland/input_method/qml.hpp

+	/// The @@Keyboard$ that will handle the grabbed keyboard.
+	/// Use @@KeyboardTextEdit$ for most cases.


You can drop the $ if there's a space, period, or comma after the reference

outfoxxed · 2025-11-21T07:24:43Z

src/wayland/input_method/qml.hpp

+	Q_INVOKABLE void
+	sendPreeditString(const QString& text, int32_t cursorBegin = -1, int32_t cursorEnd = -1);


The documentation generator doesn't properly support default args, so probably make two forms of this: sendPreeditString(text, begin, end), sendPreeditString(text)

outfoxxed · 2025-11-21T08:08:28Z

src/wayland/input_method/qml.hpp

+	/// @@hasKeyboard$
+	Q_INVOKABLE [[nodiscard]] bool hasKeyboard() const;


Property getters dont need to be invokable

outfoxxed · 2025-11-21T08:12:17Z

src/wayland/input_method/keyboard_grab.cpp

+
+	key += WAYLAND_KEY_OFFSET;
+
+#if INPUT_METHOD_PRINT


I am glad you added logs but instead of ifdefing them, use logging categories (grep for QS_LOGGING_CATEGORY and qCDebug)

outfoxxed · 2025-11-21T08:55:07Z

src/wayland/input_method/manager.cpp

+InputMethodManager* InputMethodManager::instance() {
+	// The OS should free this memory when we exit
+	static auto* instance = new InputMethodManager();
+	return instance;
+}


Comment is unnnecessary.

Return nullptr when not initialized.

outfoxxed · 2025-11-21T09:03:03Z

src/wayland/input_method/qml.cpp

+void InputMethod::sendString(const QString& text) {
+	if (!this->isActive()) return;
+
+	this->handle->commitString(text);
+	this->handle->commit();
+}
+
+void InputMethod::sendPreeditString(const QString& text, int32_t cursorBegin, int32_t cursorEnd) {
+	if (!this->isActive()) return;
+
+	this->handle->sendPreeditString(text, cursorBegin, cursorEnd);
+	this->handle->commit();
+}
+
+void InputMethod::deleteText(int before, int after) {
+	if (!this->isActive()) return;
+
+	this->handle->deleteText(before, after);
+	this->handle->commit();
+}


I would prefer commit to be separated for this one. The pre-edit string and cursor region may be better exposed as properties instead of a function as well.

outfoxxed · 2025-11-21T09:06:08Z

src/wayland/input_method/qml.cpp

+QQmlComponent* InputMethod::keyboardComponent() const { return this->mKeyboardComponent; }
+void InputMethod::setKeyboardComponent(QQmlComponent* keyboardComponent) {
+	if (this->mKeyboardComponent == keyboardComponent) return;
+	this->mKeyboardComponent = keyboardComponent;
+	emit this->keyboardComponentChanged();
+
+	if (this->keyboard) {
+		this->keyboard->deleteLater();
+		this->keyboard = nullptr;
+	}
+
+	this->handleKeyboardActive();
+}


Why are components even involved in this? Shouldn't we just expose the keyboard as a member of the inputmethod which is null if we haven't grabbed it?

outfoxxed · 2025-11-21T09:11:21Z

src/wayland/input_method/types.cpp

+QString QMLContentHint::toString(Enum contentHint) {
+	QString string = "";
+
+	bool first = true;
+
+	if (contentHint & Completion) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "Completion";
+		first = false;
+	}
+	if (contentHint & Spellcheck) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "Spellcheck";
+		first = false;
+	}
+	if (contentHint & AutoCapitalization) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "AutoCapitalization";
+		first = false;
+	}
+	if (contentHint & Lowercase) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "Lowercase";
+		first = false;
+	}
+	if (contentHint & Uppercase) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "Uppercase";
+		first = false;
+	}
+	if (contentHint & Titlecase) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "Titlecase";
+		first = false;
+	}
+	if (contentHint & HiddenText) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "HiddenText";
+		first = false;
+	}
+	if (contentHint & SensitiveData) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "SensitiveData";
+		first = false;
+	}
+	if (contentHint & Latin) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "Latin";
+		first = false;
+	}
+	if (contentHint & Multiline) {
+		if (!first) {
+			string += " | ";
+		}
+		string += "Multiline";
+		first = false;
+	}
+
+	if (string == "") string = "None";
+	return string;


Make a list and join it instead of if (!first)?

outfoxxed · 2025-11-21T09:13:18Z

src/wayland/input_method/virtual-keyboard-unstable-v1.xml

Might be better to split this

Include from w-p instead?

CMurtagh-LGTM force-pushed the cmurtagh/inputmethod branch from f07a697 to d92ced5 Compare August 24, 2025 10:30

outfoxxed force-pushed the master branch from df77d21 to 6eb1255 Compare September 4, 2025 10:21

CMurtagh-LGTM force-pushed the cmurtagh/inputmethod branch 2 times, most recently from b265756 to 7054e2d Compare September 5, 2025 05:18

CMurtagh-LGTM marked this pull request as ready for review September 5, 2025 05:36

outfoxxed force-pushed the master branch from c123799 to e9a574d Compare September 19, 2025 09:16

outfoxxed force-pushed the master branch 3 times, most recently from a92879d to 9bb2c04 Compare October 4, 2025 20:00

CMurtagh-LGTM added 3 commits November 19, 2025 12:03

wayland: input method protocol

a02513e

lint errors

2e3ebd6

add some more <cstdint>

c7ed0c2

CMurtagh-LGTM force-pushed the cmurtagh/inputmethod branch from 44f8232 to c7ed0c2 Compare November 19, 2025 01:04

CMurtagh-LGTM added 2 commits November 20, 2025 14:32

this->

5979012

Perhaps fix 6.6.0 gcc build

79ed7ee

outfoxxed requested changes Nov 21, 2025

View reviewed changes

		/// The @@Keyboard$ that will handle the grabbed keyboard.
		/// Use @@KeyboardTextEdit$ for most cases.

		Q_INVOKABLE void
		sendPreeditString(const QString& text, int32_t cursorBegin = -1, int32_t cursorEnd = -1);

		/// @@hasKeyboard$
		Q_INVOKABLE [[nodiscard]] bool hasKeyboard() const;

wayland: input method protocol #195

Are you sure you want to change the base?

wayland: input method protocol #195

Conversation

CMurtagh-LGTM commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

outfoxxed commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CMurtagh-LGTM commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CMurtagh-LGTM commented Nov 17, 2025

Uh oh!

outfoxxed commented Nov 18, 2025

Uh oh!

outfoxxed left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

CMurtagh-LGTM commented Aug 22, 2025 •

edited

Loading

outfoxxed commented Sep 3, 2025 •

edited

Loading

CMurtagh-LGTM commented Sep 3, 2025 •

edited

Loading