Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Input fields get random symbols like "[B[AA1" or "B" and others pasted in on every action #136

Closed
DavidLazarescu opened this issue Jul 3, 2021 · 28 comments · Fixed by #138

Comments

@DavidLazarescu
Copy link

Input fields get random symbols like "[B[AA1" or "B" and others pasted in on every action like typing, using the arrow keys and moving the mouse

@DavidLazarescu DavidLazarescu changed the title Input fields get random symbols like "[B[AA1" or "B" and others pasted in on every action [BUG] Input fields get random symbols like "[B[AA1" or "B" and others pasted in on every action Jul 3, 2021
@ArthurSonzogni
Copy link
Owner

Hello, what kind of terminal emulator are you using?

@ArthurSonzogni
Copy link
Owner

In particular, what sequence of byte it sent when you are using the example

./example/util/print_key_press
and use your keyboard?

You should get those numbers:

│( 27 91 65 ) -> (special)                                                                               │
│( 27 91 68 ) -> (special)                                                                               │
│( 27 91 66 ) -> (special)                                                                               │
│( 27 91 67 ) -> (special)                                                                               │

You can try it online against the xterm.js terminal emulator and check you receive the same sequences:
https://arthursonzogni.com/FTXUI/examples/?file=./util/print_key_press.js

@DavidLazarescu
Copy link
Author

It changes how extreme it is, sometimes even more symbols than here get pasted in. https://streamable.com/mg7g85
All actions make this happen, deleting, switching with arrow keys or just writing.

@AhmedMostafa16
Copy link

AhmedMostafa16 commented Jul 3, 2021

Hello, what kind of terminal emulator are you using?

Windows CMD. I have tried PowerShell, but the same.

@DavidLazarescu
Copy link
Author

Hello, what kind of terminal emulator are you using?

Windows CMD. I have tried PowerShell, but the same.

@ArthurSonzogni we are working on the same project btw, so i m using Windows CMD too. Btw, its not the same symbols your https://arthursonzogni.com/FTXUI/examples/?file=./util/print_key_press.js shows me

@DavidLazarescu
Copy link
Author

@ArthurSonzogni, do u have any idea?

@ArthurSonzogni
Copy link
Owner

I finally was able to test on Windows.
There are several problems I tried to mitigate with:
#138

However I was not able to reproduce your problems. I just saw you made a video. This help a lot. Thanks!

@DavidLazarescu
Copy link
Author

I finally was able to test on Windows.
There are several problems I tried to mitigate with:
#138

However I was not able to reproduce your problems. I just saw you made a video. This help a lot. Thanks!

What should we do now? If you check my github account, you can find my project called "PW-Manager" clone it and run it, you ll see the errors too

@ArthurSonzogni
Copy link
Owner

I tried locally on "gallery.exe", using visual studio. It seems to be working.
https://user-images.githubusercontent.com/4759106/124385624-d46da680-dcd6-11eb-9ca5-f7c46bdfe96e.mp4

Maybe this is caused by your project? Then I should try directly your project.

@AhmedMostafa16
Copy link

AhmedMostafa16 commented Jul 4, 2021

I tried locally on "gallery.exe", using visual studio. It seems to be working.
https://user-images.githubusercontent.com/4759106/124385624-d46da680-dcd6-11eb-9ca5-f7c46bdfe96e.mp4

Maybe this is caused by your project? Then I should try directly your project.

I have tried the same project on Linux, but it works well.
Edit: I have cleaned the project and tried again. I have fetched the latest changes of FTXUI, but the bug is still.

@DavidLazarescu
Copy link
Author

I tried locally on "gallery.exe", using visual studio. It seems to be working.
https://user-images.githubusercontent.com/4759106/124385624-d46da680-dcd6-11eb-9ca5-f7c46bdfe96e.mp4

Maybe this is caused by your project? Then I should try directly your project.

would be awesome if you could try to open it directly, we have one global screen which the requested file renders to, maybe this could be a problem.
(As a side question, do you provide any option to mask text, for example passwords?)

@DavidLazarescu
Copy link
Author

DavidLazarescu commented Jul 4, 2021

I tried locally on "gallery.exe", using visual studio. It seems to be working.
https://user-images.githubusercontent.com/4759106/124385624-d46da680-dcd6-11eb-9ca5-f7c46bdfe96e.mp4
Maybe this is caused by your project? Then I should try directly your project.

I have tried the same project on Linux, but it works well.
Edit: I have cleaned the project and tried again. I have fetched the latest changes of FTXUI, but the bug is still.

@ArthurSonzogni, i ve tried it on windows with the ubuntu cmd and it had the same problems

@ArthurSonzogni
Copy link
Owner

Sorry =, It was very painful for me to try your project. First install windows, then visual studio, then git, then cmake. Then discover I need vcpkg. Then discover SQLite3 was missing. Then trying to use vcpkg to install SQLite3, which errors because I apparently need to install the "english" language pack. Then install the english language pack for visual studio. Trying again installing SQLite. Then trying again cmake with the error:`

CMake Error at CMakeLists.txt:77 (add_subdirectory):
  The source directory

    C:/Users/arthur/programmation/PW-Manager/lib/clip

  does not contain a CMakeLists.txt file.

I have to abandon trying to build your project. Next time, you probably want to abandon vcpkg and directly use cmake fetchcontent for building from source everything you need.

I will take a look at your source code directly instead of trying to reproduce your problem.

@DavidLazarescu
Copy link
Author

Sorry =, It was very painful for me to try your project. First install windows, then visual studio, then git, then cmake. Then discover I need vcpkg. Then discover SQLite3 was missing. Then trying to use vcpkg to install SQLite3, which errors because I apparently need to install the "english" language pack. Then install the english language pack for visual studio. Trying again installing SQLite. Then trying again cmake with the error:`

CMake Error at CMakeLists.txt:77 (add_subdirectory):
  The source directory

    C:/Users/arthur/programmation/PW-Manager/lib/clip

  does not contain a CMakeLists.txt file.

I have to abandon trying to build your project. Next time, you probably want to abandon vcpkg and directly use cmake fetchcontent for building from source everything you need.

I will take a look at your source code directly instead of trying to reproduce your problem.

If i can help you in any way, just tell me. Thanks for your quick replys btw

@ArthurSonzogni
Copy link
Owner

Sorry, I looked too quickly. You don't use vcpkg and the missing file in lib/clip is just because I need to ask git to clone recursively.

@DavidLazarescu
Copy link
Author

Sorry, I looked too quickly. You don't use vcpkg and the missing file in lib/clip is just because I need to ask git to clone recursively.

so does it work now?

@AhmedMostafa16
Copy link

Sorry, I looked too quickly. You don't use vcpkg and the missing file in lib/clip is just because I need to ask git to clone recursively.

@ArthurSonzogni you can use "git update submodule --init --recursive"

@ArthurSonzogni
Copy link
Owner

Yes, I just tried on Linux. Working very well. Nice app!
I now need to got on Windows.

@DavidLazarescu
Copy link
Author

Yes, I just tried on Linux. Working very well. Nice app!
I now need to got on Windows.

Thanks for your positive feedback, would be awesome if you could help us with also getting it working on windows

@AhmedMostafa16
Copy link

Yes, I just tried on Linux. Working very well. Nice app!
I now need to got on Windows.

@ArthurSonzogni
To build it on windows:
vcpkg install sqlite3
git update submodule --init --recursive
then build from your Visual Studio 19

@ArthurSonzogni
Copy link
Owner

Gravité	Code	Description	Projet	Fichier	Ligne	État de la suppression
Erreur		CMake Error at C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/CMakeDetermineSystem.cmake:129 (message):
  Could not find toolchain file:
  D:\src\vcpkg\scripts\buildsystems\vcpkg.cmake		C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/Common7/IDE/CommonExtensions/Microsoft/CMake/CMake/share/cmake-3.20/Modules/CMakeDetermineSystem.cmake	129	

@ArthurSonzogni
Copy link
Owner

@AhmedMostafa16
Copy link

This is a binary file with the latest changes of FTXUI.

pw-manager.zip

@ArthurSonzogni
Copy link
Owner

Okay, I found it.

What you see are the windows terminal replying to the request about reporting the cursor position relatively to the screen. This isn't really useful in fullscreen mode, because it will always report (1,1). This is useful with ScreenInteractive::TerminalOutput() for instance.

So the windows terminal should output ^[1;1R
However due to a bug:
microsoft/terminal#7583
and race conditions, it ends up outputting corrupted:

^[^[1;1R[1;1R
^^      ^^^^^ First CPR
  ^^^^^^ Second CPR

I will add one more time "mitigation" so that this bug won't affect FTXUI most of the time.

@AhmedMostafa16
Copy link

Okay, thank you for your time and efforts @ArthurSonzogni

@DavidLazarescu
Copy link
Author

DavidLazarescu commented Jul 4, 2021

Okay, I found it.

What you see are the windows terminal replying to the request about reporting the cursor position relatively to the screen. This isn't really useful in fullscreen mode, because it will always report (1,1). This is useful with ScreenInteractive::TerminalOutput() for instance.

So the windows terminal should output ^[1;1R
However due to a bug:
microsoft/terminal#7583
and race conditions, it ends up outputting corrupted:

^[^[1;1R[1;1R
^^      ^^^^^ First CPR
  ^^^^^^ Second CPR

I will add one more time "mitigation" so that this bug won't affect FTXUI most of the time.

this is awesome, thanks a lot. Would it reduce it a lot, like 90% or smth or would it just help a little? When do you think u'll have the time to implement it?

ArthurSonzogni added a commit that referenced this issue Jul 4, 2021
I finally got access to a computer using the Microsoft's Windows OS.
That's the opportunity to find and mitigate all the problems
encountered. This patch:

1. Introduce an option and a C++ definition to enable fallback for
   Microsoft's terminal emulators. This allows me to see/test the
   Microsoft output from Linux. This also allows Windows users to remove
   the fallback and target non Microsoft terminals on Windows if needed.

2. Microsoft's terminal suffer from a race condition bug when reporting
   the cursor position:
   #136
   The mitigation is not to ask for the cursor position in fullscreen
   mode where it isn't really needed and request it less often.
   This fixes: #136

3. Microsoft's terminal do not handle properly hidding the cursor. Instead
   the character under the cursor is hidden, which is a big problem. As
   a result, we don't enable setting the cursor to the best position for
   [input method editors](https://en.wikipedia.org/wiki/Input_method),
   It will be displayed at the bottom right corner.
   See:
   - microsoft/terminal#1203
   - microsoft/terminal#3093

4. Microsoft's terminals do not provide a way to query if they support
   colors. As a fallback, assume true colors is supported.
   See issue:
   - microsoft/terminal#1040
   This mitigates:
   - #135

5. The "cmd" on Windows do not properly report its dimension. Powershell
   works correctly. As a fallback, use a 80x80 size instead of 0x0.

6. There are several dom elements and component displayed incorrectly,
   because the font used is missing several unicode glyph. Use
   alternatives or less detailled one as a fallback.
ArthurSonzogni added a commit that referenced this issue Jul 4, 2021
I finally got access to a computer using the Microsoft's Windows OS.
That's the opportunity to find and mitigate all the problems
encountered. This patch:

1. Introduce an option and a C++ definition to enable fallback for
   Microsoft's terminal emulators. This allows me to see/test the
   Microsoft output from Linux. This also allows Windows users to remove
   the fallback and target non Microsoft terminals on Windows if needed.

2. Microsoft's terminal suffer from a race condition bug when reporting
   the cursor position:
   microsoft/terminal#7583.
   The mitigation is not to ask for the cursor position in fullscreen
   mode where it isn't really needed and request it less often.
   This fixes: #136

3. Microsoft's terminal do not handle properly hidding the cursor. Instead
   the character under the cursor is hidden, which is a big problem. As
   a result, we don't enable setting the cursor to the best position for
   [input method editors](https://en.wikipedia.org/wiki/Input_method),
   It will be displayed at the bottom right corner.
   See:
   - microsoft/terminal#1203
   - microsoft/terminal#3093

4. Microsoft's terminals do not provide a way to query if they support
   colors. As a fallback, assume true colors is supported.
   See issue:
   - microsoft/terminal#1040
   This mitigates:
   - #135

5. The "cmd" on Windows do not properly report its dimension. Powershell
   works correctly. As a fallback, use a 80x80 size instead of 0x0.

6. There are several dom elements and component displayed incorrectly,
   because the font used is missing several unicode glyph. Use
   alternatives or less detailled one as a fallback.
@DavidLazarescu
Copy link
Author

@ArthurSonzogni, thanks for your help! Btw, one more off topic question. Do you provide any way to mask input? for example passwords?

ArthurSonzogni added a commit that referenced this issue Jul 4, 2021
I finally got access to a computer using the Microsoft's Windows OS.
That's the opportunity to find and mitigate all the problems
encountered. This patch:

1. Introduce an option and a C++ definition to enable fallback for
   Microsoft's terminal emulators. This allows me to see/test the
   Microsoft output from Linux. This also allows Windows users to remove
   the fallback and target non Microsoft terminals on Windows if needed.

2. Microsoft's terminal suffer from a race condition bug when reporting
   the cursor position:
   microsoft/terminal#7583.
   The mitigation is not to ask for the cursor position in fullscreen
   mode where it isn't really needed and request it less often.
   This fixes: #136

3. Microsoft's terminal do not handle properly hidding the cursor. Instead
   the character under the cursor is hidden, which is a big problem. As
   a result, we don't enable setting the cursor to the best position for
   [input method editors](https://en.wikipedia.org/wiki/Input_method),
   It will be displayed at the bottom right corner.
   See:
   - microsoft/terminal#1203
   - microsoft/terminal#3093

4. Microsoft's terminals do not provide a way to query if they support
   colors. As a fallback, assume true colors is supported.
   See issue:
   - microsoft/terminal#1040
   This mitigates:
   - #135

5. The "cmd" on Windows do not properly report its dimension. Powershell
   works correctly. As a fallback, use a 80x80 size instead of 0x0.

6. There are several dom elements and component displayed incorrectly,
   because the font used is missing several unicode glyph. Use
   alternatives or less detailled one as a fallback.
@ArthurSonzogni
Copy link
Owner

This doesn't exist, but I believe I should add an option for that:
#139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants