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

Never update the local text while the user is typing #183

Open
fregante opened this issue Jan 28, 2021 · 7 comments
Open

Never update the local text while the user is typing #183

fregante opened this issue Jan 28, 2021 · 7 comments

Comments

@fregante
Copy link
Owner

  1. User types in the browser
  2. The editor sends some text (like in Sublime Text echoes the same received text #180)
  3. The browser overrides whatever the user types with what the editor sends.

This is a mistake. If you're typing somewhere, that's the only version that matters. External messages should be ignored. I attempted to fix this with faa5a8d but it caused #181.

This applies to both parts (browser or editor) but it's probably easier to avoid both issues in the browser:

  • do not echo received text back to the editor (I think this is already done)
  • ignore text from the editor while the field is focused
@subnut
Copy link
Contributor

subnut commented Feb 23, 2021

  • do not echo received text back to the editor (I think this is already done)

I can confirm. This is an excerpt from my plugin's log -

Tuesday, 23 February 2021, 14:26:02
$NVIM_LISTEN_ADDRESS: /tmp/nvimgDohIm/0
binary v0.0.42
Servers started
[14:26:03]: /tmp/nvimgDohIm/0 session closed
[14:26:28]: Focus /tmp/nvimjtWyb8/0
[14:27:32]: Connected 127.0.0.1:43740 to /tmp/nvimjtWyb8/0
[14:27:32]: 43740 got {"title":"GhostText test page","url":"fregante.github.io","syntax":"","text":"This is a second textarea","selections":[{"start":25,"end":25}]}
[14:27:42]: 43740 sent {"text": "This is a second textareaa", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaal", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaals", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalsk", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskd", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskdj", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskdjf", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskdjfl", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskdjfla", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskdjflak", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskdjflaks", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskdjflaksj", "selections": []}
[14:27:42]: 43740 sent {"text": "This is a second textareaalskdjflaksjf", "selections": []}

We can see that there is only one entry of got, that is the first WebSocket message to initiate the new editor tab. All the subsequent edits in the editor are shown by sent, and we can see that there are no got corresponding to the sent, i.e. the browser doesn't echo back the text.

@fregante
Copy link
Owner Author

fregante commented Feb 23, 2021

Thank you! As a solution for this issue, I think an easy fix would be to ignore events received within the next 500ms of a native input (which the browser extension already detects and handles correctly)

Could be as easy as:

// on verified input
clearTimeout(localEditTimer)
localEditTimer = setTimeout(() ={localEditTimer = false}, 500)

// on received input
if (localEditTimer) return

The alternative would be to constantly keep track of the browser’s focus state in the background page so that it’s available synchronously when GT received a server event.

@subnut
Copy link
Contributor

subnut commented Feb 23, 2021

I am a total noob in JS, so as far as I understood, it implements a timeout, and rejects everything it receives till the timeout expires.
if that's true, then the messages that come after the 500ms timeout shall still cause the issue!

But, overall, I still think that this is not a valid issue. The extension does what it is meant to do according to the protocol. Receive the WS message and change the text field accordingly.

I personally think the issue of not echoing the sent text is the responsibility of the editor plugin. But I can't speak for other plugin's authors.

@fregante
Copy link
Owner Author

fregante commented Feb 23, 2021

I mean, you’re not wrong.

Maybe the extension should highlight this editor mistake on the testing page. If it receives a message identical to the one just sent, show a notification.

This way it would be easier for editor plugin authors to find and solve their issues, or simply include a protocol tester that does so automatically in the options page.

@subnut
Copy link
Contributor

subnut commented Feb 23, 2021

BTW, I just tried to reproduce #180, but couldn't see the effect. Is there any kind of logging facility available to investigate?
Also, could you please check if #180 is still happening?

@fregante
Copy link
Owner Author

fregante commented Feb 23, 2021

There should be some logging in the background page, but I don’t remember exactly if it makes this visible.

This usually isn’t a problem with short text because it’s quick enough that the content still matches, but if you use longer content and type fast in the browser you’ll see that some keypresses will be lost/overridden.

@subnut
Copy link
Contributor

subnut commented Feb 23, 2021

if you use longer content

Ah, right! 🤦
Tried with longer content, and yes, it occurs.


Could you grant me push access to the sublime extension repo? I will start a new branch and see what I can do.

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

No branches or pull requests

2 participants