FvwmForm: process UTF-8 input and paste request in input fields #1212

ONykyf · 2025-06-13T06:43:11Z

To make FvwmForm input fields UTF-8 aware, input.size field is duplicated as input.width, the first one containing size in bytes and the latter width in multibyte chars. To allow walking along a string with chars of different lengths (1 to 4 bytes), e.g., for pasting or moving a cursor by arrow keys or by a mouse click, a helper function find_nth_UTF8_char() is introduced.

Fixes #1211

To make FvwmForm input fields UTF-8 aware, input.size field is duplicated as input.width, the first one containing size in bytes and the latter width in multibyte chars. To allow walking along a string with chars of different lengths (1 to 4 bytes), e.g., for pasting or moving a cursor by arrow keys or by a mouse click, a helper function find_nth_UTF8_char() is introduced.

…ke this into account on restart.

…ve an ineffective assignment

ONykyf · 2025-06-15T05:57:55Z

@ThomasAdam @somiaj No more places affected by "multibyteness" found. Testing shows no problems. Please review.

ThomasAdam · 2025-06-16T16:41:55Z

modules/FvwmForm/FvwmForm.c

@@ -2135,10 +2143,17 @@ char* find_nth_UTF8_char(char *str, char *before,
    }

    while (1) {
-	if (*str == '\0' || (before && str >= before)
+	if (*str == '\0' || l == 0


1 == 0? We can do better than this, @ONykyf

A simple way to signal that no valid UTF-8 char can be found at the requested position, so no need for additional Boolean flags, and convenient to use in conditions.

A simple way to signal that no valid UTF-8 char can be found at the requested position, so no need for additional Boolean flags, and convenient to use in conditions.

No... it's just unnecessary.

Please remove this instance, and subsequent ones.

ThomasAdam · 2025-06-16T16:42:11Z

modules/FvwmForm/FvwmForm.c

 		    pstr = str;
+		    l = 0;
+		}
+		else if (l == 0 || i == 0) {


ThomasAdam · 2025-06-16T16:43:53Z

modules/FvwmForm/FvwmForm.c

@@ -947,10 +947,11 @@ static void ct_Input(char *cp)
  item->input.blanks = fxmalloc(item->input.width);
  for (j = 0; j < item->input.width; j++)
    item->input.blanks[j] = ' ';
-  item->input.buf = strlen(item->input.init_value) + 1;
+  item->input.buf = strlen(item->input.init_value) + 1; /* room for init value */


No need for this comment.

ThomasAdam · 2025-06-16T16:45:43Z

modules/FvwmForm/FvwmForm.c

-	strcpy(item->input.init_value,cp); /* new initial value in field */
+	strncpy(item->input.init_value, cp, var_len); /* new initial value */
+	item->input.init_value[var_len] = '\0';


I'd rather see xasprintf or similar used here, rather than a change to strncpy.

We need to copy a part of a string up to a certain position (not necessarily to '\0'), so strndup() seems to be the best candidate. In case strndup() fails, the subsequent fxstrdup() will catch this.

ThomasAdam · 2025-06-16T16:50:20Z

@ONykyf -- Thanks for this. It looks OK, although I'm not going to get time to properly look at it until later on the week.

Other than my very small nits I've identified, I think we can probably improve on the logic in a few areas, but I'll wait until I've had change to go over those properly.

…ocate mouse click faster, and translate labels with Gettext

ThomasAdam · 2025-06-21T13:13:00Z

@ONykyf

This PR is getting there, still with comments from me which need addressing.

I'm still not completely happy with the state of things overall though, but it's a convoluted mess and this is probably going to have to be good enough, but I suspect I'll merge this once you've finished addressing things, and then tidy it up afterward.

ONykyf · 2025-06-21T15:33:28Z

Hope now the logic of the helper function is more straightforward and manageable enough by whoever will do this in the future.

ThomasAdam · 2025-06-16T16:48:05Z

modules/FvwmForm/FvwmForm.c

+#if 0   /* no UTF-8, single-byte locale */
+	pstr = str;
+	str++;
+	i++;
+#else   /* parse UTF-8 string */


May as well just remove the #if 0/#else stuff...

ThomasAdam · 2025-06-21T13:06:20Z

modules/FvwmForm/FvwmForm.c

@@ -2135,10 +2143,17 @@ char* find_nth_UTF8_char(char *str, char *before,
    }

    while (1) {
-	if (*str == '\0' || (before && str >= before)
+	if (*str == '\0' || l == 0


A simple way to signal that no valid UTF-8 char can be found at the requested position, so no need for additional Boolean flags, and convenient to use in conditions.

No... it's just unnecessary.

Please remove this instance, and subsequent ones.

ThomasAdam · 2025-06-21T13:09:15Z

modules/FvwmForm/FvwmForm.c

-	strcpy(item->input.init_value,cp); /* new initial value in field */
+	strncpy(item->input.init_value, cp, var_len); /* new initial value */
+	item->input.init_value[var_len] = '\0';


ThomasAdam · 2025-06-21T13:10:43Z

modules/FvwmForm/FvwmForm.c

+	if (num) *num = -1;
+	if (len) *len = 0;


Style: assignment after if() should be on its own line.

ThomasAdam · 2025-06-21T13:10:57Z

modules/FvwmForm/FvwmForm.c

+		if (num) *num = i - 1;
+		if (len) *len = l;


ThomasAdam · 2025-06-21T17:17:04Z

modules/FvwmForm/FvwmForm.c

+	pstr = str;
+	l = 0;
+    }
+    else if ((l == 0) || (*str == '\0')) { /* invalid UTF-8 char or '\0' */


We said we weren't going to do this 1 == 0 business....

ThomasAdam · 2025-06-21T17:17:42Z

modules/FvwmForm/FvwmForm.c

+    if (num) *num = i - 1;
+    if (len) *len = l;


Style, put the conditionals on their own line, after if()...

ONykyf · 2025-06-21T18:05:00Z

Removed.

ThomasAdam

This can be squashed into a previous commit as it's not introducing anything new.

ONykyf · 2025-06-28T08:03:46Z

@ThomasAdam Unsafe strndup() replaced with fxstrndup() analogous to fxstrdup() but truncated if necessary.

In fact getpwuid.c also contains strndup() alongside fxstrdup(). Maybe replace it there as well?

ThomasAdam · 2025-06-28T11:11:19Z

@ThomasAdam Unsafe strndup() replaced with fxstrndup() analogous to fxstrdup() but truncated if necessary.

No thanks. I don't think we need this.

In fact getpwuid.c also contains strndup() alongside fxstrdup(). Maybe replace it there as well?

getpwuid.c is a compat file found elsewhere, it's not changing to fit in with any wrapper functions.

The point of the fx*() wrappers is mostly for portability; fxmalloc() is perhaps the only exception to this.

You may as well just remove this latest commit, and make the following adjustment:

item->input.init_value = fxmalloc(var_l + 1);
strlcpy(...);

I also think we need to look at item->input_init_value vs item->input.value.

ONykyf · 2025-06-28T11:24:31Z

@ThomasAdam Unsafe strndup() replaced with fxstrndup() analogous to fxstrdup() but truncated if necessary.
You may as well just remove this latest commit, and make the following adjustment:
item->input.init_value = fxmalloc(var_l + 1);
strlcpy(...);

Done

I also think we need to look at item->input_init_value vs item->input.value.

item->input_init_value is used to restore item->input.value on form restarts.

ONykyf added 2 commits June 13, 2025 09:38

FvwmForm: Add also a check that init values are correct UTF-8, and ta…

ea426a0

…ke this into account on restart.

ONykyf force-pushed the on/fvwmform-utf8 branch from 1ec6cde to ea426a0 Compare June 13, 2025 09:33

ONykyf added 2 commits June 14, 2025 11:22

FvwmForm: Clarify comments, improve handling of invalid strings, remo…

94c2afc

…ve an ineffective assignment

FvwmForm: Count UTF-8 characters in initial values

021dd2a

ThomasAdam requested changes Jun 16, 2025

View reviewed changes

FvwmForm: Remove an unnecessary comment, improve memory allocation, l…

eafc39e

…ocate mouse click faster, and translate labels with Gettext

ONykyf requested a review from ThomasAdam June 21, 2025 17:01

ThomasAdam reviewed Jun 21, 2025

View reviewed changes

ONykyf requested a review from ThomasAdam June 21, 2025 18:04

ThomasAdam requested changes Jun 21, 2025

View reviewed changes

FvwmForm: Clarify logic in a helper function

c18fd01

ONykyf force-pushed the on/fvwmform-utf8 branch from 3dd4dca to c18fd01 Compare June 21, 2025 18:42

ONykyf requested a review from ThomasAdam June 21, 2025 18:44

FvwmForm: Replace strndup() with fxmalloc()+strlcpy()

53d8c40

ONykyf force-pushed the on/fvwmform-utf8 branch from 7611217 to 53d8c40 Compare June 28, 2025 11:22

Uh oh!

FvwmForm: process UTF-8 input and paste request in input fields #1212

Are you sure you want to change the base?

FvwmForm: process UTF-8 input and paste request in input fields #1212

Uh oh!

Conversation

ONykyf commented Jun 13, 2025

Uh oh!

ONykyf commented Jun 15, 2025

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!

ONykyf Jun 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThomasAdam commented Jun 16, 2025

Uh oh!

ThomasAdam commented Jun 21, 2025

Uh oh!

ONykyf commented Jun 21, 2025

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!

ONykyf commented Jun 21, 2025

Uh oh!

ThomasAdam left a comment

Choose a reason for hiding this comment

Uh oh!

ONykyf commented Jun 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThomasAdam commented Jun 28, 2025

Uh oh!

ONykyf commented Jun 28, 2025

Uh oh!

Uh oh!

ONykyf Jun 17, 2025 •

edited

Loading

ONykyf commented Jun 28, 2025 •

edited

Loading