Fix teleport when exiting out of alt buffer. v2

[anthonykim1]

Had people try out: microsoft/vscode#224750 (comment)

Apparently #5390 wasn't enough.
If you really interact scroll around alt buffer, after maxing out in normal buffer, you can sometimes repro this if you get lucky.

@Tyriar The teleport issue came back, this time it was more difficult to repro, but I was able to repro.

Steps to repro for me was:

1. fill in bunch of things in the normal buffer with echo -l {1..10000}
2. enter alt buffer,
3. scroll very aggressively in alt buffer with+without content
4. Exit vim via :q!
5. notice scroll is at the top again. You have to get pretty lucky. I noticed trying to max out my normal buffer, and also filling in content inside alt buffer & scrolling aggressively there, back <-> forth between helped increase the probability of repro. Sometimes had to launch vim multiple times and exit out re-doing the steps.

---

I spent some time and it seems like at certain point undefined !== 1000 causes true for

  if (ydisp !== this._latestYDisp) {
      this._scrollableElement.setScrollPosition({
        scrollTop: ydisp * this._renderService.dimensions.css.cell.height
      });

when ydisp is undefined after exiting out of alt buffer. Hence scroll will be brought to very top.

• undefined above is likely coming from

  xterm.js/src/browser/Viewport.ts

  Line 97 in a1d8e96

  this._register(this._bufferService.buffers.onBufferActivate(() => this.queueSync()));

EDIT: ydisp cannot be undefined in sync due to #5411 (comment)

Adding this._latestYDisp = ydispin _sync similar to queueSync seems to solve the problem.

Here are some logs PRE-PR:

(Entering vim)

(Exit out of vim)

(start scrolling)

[Tyriar]

Does it work if we set this only when this._latestYDisp is undefined? Normally _latestYDisp is set when handling scroll events, not syncing.

[anthonykim1]

I just tested this with conditional this._latestYDisp is undefined , and it's not working..

this._latestYDisp switches from 0 to 1000 in queueSync, and this this._latestYDisp passes down and becomes ydisp in sync where

 if (ydisp !== this._latestYDisp) {    // 1. this would be 0 !== 1000
      this._scrollableElement.setScrollPosition({
        scrollTop: ydisp * this._renderService.dimensions.css.cell.height   // 2. ydisp is 0.
      });
    }

Afterwards, when latestYDisp is 1000, and ydisp is also 1000, we don't get to call

  scrollTop: ydisp * this._renderService.dimensions.css.cell.height  

because if (ydisp !== this._latestYDisp) is false, and this would leave the scrollTop to remain 0, instead updating to correct value (end of buffer)
---> Hence you get stuck in 0 scrollTop position.

Also, _sync 's ydisp cannot actually be undefined since it has default value, from:

 private _sync(ydisp: number = this._bufferService.buffer.ydisp): void {

Long story short,
In the "teleport" scenario, I think the the problem appears because we don't get to update scrollTop to the correct place when we go from having 0 to 1000 ydisp when calling _sync

Maybe this is also some race condition, as I can only repro sometime

[Tyriar]

Does this._bufferService.buffers.onBufferActivate need to be handled specially? It seems like it's related to buffer switching during a smooth scroll happening. You could try increasing smooth scroll duration significantly (10000ms?) to see if it's easier to repro?

[anthonykim1]

Yeah, maybe more proper/intuitive fix is that we set latestYDisp to undefined onBufferActivate.
This way we prevent from any contamination when switching between buffers.

Setting the smooth scroll to 50000 and also aggressively scrolling and letting the mouse scroll run right before exiting out via q! seemed to help me repro more consistently.

[jerch]

The new vs scroller integration still has some issues get updated correctly, maybe it is related to this here? Plz see #5373
and #5339.