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

Work around JuiceSSH rendering bug #867

Closed
wants to merge 1 commit into from

Conversation

cgull
Copy link
Member

@cgull cgull commented Mar 27, 2017

JuiceSSH apparently has a bug where ECH for one character (ESC [ 1 X) does
not actually erase the character, in its code that receives and interprets
Mosh state updates. This was hidden before because Mosh <= 1.2.5 never sent
this sequence, it sent ESC [ X instead as an optimization.

Do the better optimization of sending spaces for short sequences of blanks
instead.

This appears to fix #846.

JuiceSSH apparently has a bug where ECH for one character (ESC [ 1 X) does
not actually erase the character, in its code that receives and interprets
Mosh state updates.  This was hidden before because Mosh <= 1.2.5 never sent
this sequence, it sent ESC [ X instead as an optimization.

Do the better optimization of sending spaces for short sequences of blanks
instead.
@keithw
Copy link
Member

keithw commented Mar 27, 2017

It's been a while since I had this swapped in, but my recollection is that we always want to use ECH on systems that support it, even for short runs of blank cells, because there's a semantic difference (for copy-and-paste purposes) between overwriting a cell with a space character and erasing it. You may have more recent state though...

@cgull
Copy link
Member Author

cgull commented Mar 27, 2017

This only affects what we do for blanks between two printing characters, not at the end of the line. Is ECH important in the middle of a line too?

@keithw
Copy link
Member

keithw commented Mar 27, 2017

I don't know, to be honest, although I think you're right that it's fine within a line.

@cgull
Copy link
Member Author

cgull commented Mar 27, 2017

Ah, comment confusion here. I think @keithw and I agree that ECH is unimportant mid-line, though.

@lahwran
Copy link

lahwran commented Apr 17, 2017

looking forward to this fix! this has irritated me for a while. anything security-related preventing this from being merged into the nightlies?

@cgull
Copy link
Member Author

cgull commented Apr 25, 2017

Pulled. No, there wasn't anything security related stopping me, merely inattention and lack of focus on Mosh-- I've not been my best lately.

@cgull cgull closed this Apr 25, 2017
@cgull cgull deleted the juicessh-fix branch August 1, 2017 23:17
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 this pull request may close these issues.

juicessh not compatible with current mosh-server
3 participants