Correct on-screen keyboard behaviour for Arabic characters

[AhmedAdelAbouzeid]

Description of changes and context

1- Prevent Arabic characters from being dis-joined when using the on-screen keyboard
2- Prevent the flipping of existing Arabic text when editing (ex. renaming something) using the on-screen keyboard

Both the above points are cleared by turning off character flipping in calls to utf8ToW charset conversion function for ACTION_INPUT_TEXT and GUI_MSG_SET_TEXT (SetLabel2 function)

This fixes #16040

How Has This Been Tested?

The changes were tested using Kodi on Windows 10 x64 (1909)
The on-screen keyboard was tested using a RTL language (Arabic) and a LTR language (English) and behave correctly for both languages after the changes. Kodi's interface language was alternated between Arabic and English during the test and didn't affect the results.

Screenshots

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• None of the above (please explain below)

Checklist:

• My code follows the Code Guidelines of this project
• My change requires a change to the documentation, either Doxygen or wiki
• I have updated the documentation accordingly
• I have read the Contributing document
• I have added tests to cover my change
• All new and existing tests passed

[phunkyfish]

@AhmedAdelAbouzeid can you please prefix the commit message with what the change does. I.e. a speaking description.

Other than that it looks good to me, will also ask one of the skin guys.

[phunkyfish]

@ronie this looks reasonable.

[ronie]

screenshots look nice, but i have no idea what the code changes do.
this is far out of my league :-)

[phunkyfish]

screenshots look nice, but i have no idea what the code changes do.
this is far out of my league :-)

No worries, thanks for taking a look.

[phunkyfish]

@MilhouseVH can we put this in a testbuild and see if any side effects are reported?

[MilhouseVH]

Will do, should be in tonight's #0419 build.

[AhmedAdelAbouzeid]

Thanks for reviewing. I changed the commit message's title.

The code changes are limited to the edit control and only affect RTL languages. The changes ultimately bypass calls to FriBiDi which doesn't do anything to LTR languages anyway unless they are mixed with RTL languages in the same line.

I will test these cases (mixed RTL/LTR) and text including numerical values and signs later today.

[phunkyfish]

Thanks @AhmedAdelAbouzeid

[AhmedAdelAbouzeid]

I tested the keyboard with mixed RTL/LTR languages, numbers and signs and found no issues with characters getting separated nor reversed.

While testing I noticed that if the cursor is placed in the middle of an Arabic word, the characters to the left of the cursor are moved to the start of the word while the cursor is in its hidden state and then the characters return to their correct position again when the cursor is visible. This is easily fixed by modifying the character used to indicate the hidden cursor state to a simple space or null character.

Is it OK to add a commit for this to the pull request ?

[phunkyfish]

Yes of course. Please add the commit.

[AhmedAdelAbouzeid]

I will work on adjusting the arrow buttons as the cursor movement inside RTL text is reversed (clicking the left arrow moves the cursor right and vice versa). The code for the buttons' actions currently manipulate the cursor index without checking the text direction first.
The Arabic keyboard is fairly usable now though, I hope other changes can be done in a new PR.

[phunkyfish]

Yes, i think other changes in a new PR is fine.

Let's just check with @MilhouseVH if there is any feedback on this PR. Otherwise we can push it in.

[phunkyfish]

Can you explain this change?

[AhmedAdelAbouzeid]

For the blinking effect, the cursor will alternate between '|' and ' ' (simple space - 0x20) instead of between '|' and '𰁼' (It does look like a space in some fonts - 0x3007C)

The '𰁼' while looking like a space (or a double space), seem to reverse Arabic words around it even in word processors (If the paragraph direction is LTR).

[phunkyfish]

Great, thanks. Your explanation makes sense.

Although I’m not sure I understand why that was chosen for the original code! A mystery.

[MilhouseVH]

No feedback so far (none good, none bad). All I can say with any certainty is that it doesn't appear to have broken anything!

[phunkyfish]

That will do, thanks!

[phunkyfish]

LGTM

[phunkyfish]

Thanks @AhmedAdelAbouzeid for the contribution!

[phunkyfish]

@AhmedAdelAbouzeid

I think I found out why that special character was used for a space. Without it if you change a text setting the text will move left/right on each blink.

Is it possible to only use a real space for arabic and similar languages and use the old character otherwise?

[AhmedAdelAbouzeid]

This is because the character width for ' ' and '|' may be different depending on the used font; for Arial they have the same width but this is not the case for the default font.
Using a figure space (U+2007) instead of basic space eliminates text movement for the default font but causes movement when using Arial.

I will work at solving this hopefully without going for language-specific nor font-specific solutions.

[phunkyfish]

Amazing, thanks!

FYI @AlwinEsch

[AhmedAdelAbouzeid]

PR #17790 solves the above issue, please check it. Thanks!