terminal: bump xterm and addons

[vince-fugnitto]

What it does

The pull-request bumps xterm and related addons to their latest versions to benefit from additional improvements and bug fixes.

How to test

1. start the example application(s)
2. spawn a terminal - confirm it works as expected
3. confirm that resizing and searching within the terminal works as expected
4. confirm that split terminal works as expected
5. confirm that updating terminal related preferences takes effect

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[msujew]

The change works well on Chromium based browsers, but breaks on FireFox, since we do a bit of monkey-patching:

theia/packages/terminal/src/browser/terminal-widget-impl.ts

Lines 644 to 661 in abc4578

	const renderService: any = (this.term as any)._core._renderService;
	const originalFunc: (entry: IntersectionObserverEntry) => void = renderService._onIntersectionChange.bind(renderService);
	const replacement = function (entry: IntersectionObserverEntry): void {
	if (entry.target.ownerDocument !== document) {
	// in Firefox, the intersection observer always reports the widget as non-intersecting if the dom element
	// is in a different document from when the IntersectionObserver started observing. Since we know
	// that the widget is always "visible" when in a secondary window, so we mark the entry as "intersecting"
	const patchedEvent: IntersectionObserverEntry = {
	...entry,
	isIntersecting: true,
	};
	originalFunc(patchedEvent);
	} else {
	originalFunc(entry);
	}
	};
	renderService._onIntersectionChange = replacement;

_onIntersectionChange doesn't seem to be a method in the renderService anymore.

[msujew]

I was able to get it to work again by replacing the existing code with this:

const renderService: any = (this.term as any)._core._renderService;
const originalFunc: (entry: IntersectionObserverEntry) => void = renderService._handleIntersectionChange.bind(renderService);
const replacement = function (entry: IntersectionObserverEntry): void {
    if (entry.target.ownerDocument !== document) {
        // in Firefox, the intersection observer always reports the widget as non-intersecting if the dom element
        // is in a different document from when the IntersectionObserver started observing. Since we know
        // that the widget is always "visible" when in a secondary window, so we refresh the rows ourselves
        renderService._pausedResizeTask.flush();
        renderService.refreshRows(0, renderService._rowCount - 1);
    } else {
        originalFunc(entry);
    }
};

renderService._handleIntersectionChange = replacement;

[vince-fugnitto]

I was able to get it to work again by replacing the existing code with this:

@msujew good catch and thank you for the suggestion! I'll add you as a co-author whenever we merge :)

[msujew]

I'll tentatively approve. There is still one edge case on FireFox that seems to experience a regression: When maximizing a secondary terminal widget, the renderer becomes quite sluggish, only updating every few seconds.

However, I couldn't figure out what causes it or how to resolve it, so I'll approve anyway. I think it's a rare enough issue. Maybe @tsmaeder can chime in and see how to fix it?

[tsmaeder]

@msujew if I look at the relevant code change in RendererService, the method _onIntersectionChange has simply been renamed to _handleIntersectionChange. My suggestion would be to just adjust the method name, but leave the rest of the monkey-patch alone: you're proposal does refreshes unconditionally, which might well lead to the degraded performance you're observing. Was there a reason you changed more than just the method name?

[msujew]

Was there a reason you changed more than just the method name?

Yep, just using the correct method fixed the error in the console, but terminals still experienced a render issue. Any terminal didn't re-render anymore after popping it out to a secondary window.

[tsmaeder]

I think this is worth investigating: if you look at the code in RendererService.ts, I think the only time the _isPaused flag is set is in the _handleIntersectionChange method, which means _needsFullRefresh is never reset to false. We're effectively turning off incremental paints, just at a glance. That doesn't sound like the right thing to do.

[tsmaeder]

Putting a block on this one so nobody accidentally merges it: I believe we have a performance problem with incremental updates after the library update.

[tsmaeder]

@msujew can you remember what the exact rendering issue was on firefox? For me, there are rendering issues when I resize the external window, but they are the same on chrome and on firefox.
@vince-fugnitto mind if I take over this one?

[vince-fugnitto]

@vince-fugnitto mind if I take over this one?

Sure, go ahead!

Can't review my own changes

[tsmaeder]

@msujew I fixed the tests. Could you give this one another review?

[msujew]

Yep, perfect. Works good now. No sluggishness on Firefox anymore.