Skip to content

Commit

Permalink
Merge pull request xtermjs#4798 from tisilent/fix-domrenderer-isCurso…
Browse files Browse the repository at this point in the history
…rInitialized

using isCursorInitialized for domrenderer
  • Loading branch information
Tyriar authored Sep 14, 2023
2 parents bdb25c2 + 2f9439b commit 1618092
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 7 deletions.
3 changes: 2 additions & 1 deletion addons/xterm-addon-canvas/test/CanvasRenderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import test from '@playwright/test';
import { ISharedRendererTestContext, injectSharedRendererTests } from '../../../out-test/playwright/SharedRendererTests';
import { ISharedRendererTestContext, injectSharedRendererTests, injectSharedRendererTestsStandalone } from '../../../out-test/playwright/SharedRendererTests';
import { ITestContext, createTestContext, openTerminal } from '../../../out-test/playwright/TestUtils';

let ctx: ITestContext;
Expand All @@ -28,4 +28,5 @@ test.describe('Canvas Renderer Integration Tests', () => {
test.skip(({ browserName }) => browserName === 'webkit');

injectSharedRendererTests(ctxWrapper);
injectSharedRendererTestsStandalone(ctxWrapper);
});
3 changes: 2 additions & 1 deletion addons/xterm-addon-webgl/test/WebglRenderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import test from '@playwright/test';
import { ISharedRendererTestContext, injectSharedRendererTests } from '../../../out-test/playwright/SharedRendererTests';
import { ISharedRendererTestContext, injectSharedRendererTests, injectSharedRendererTestsStandalone } from '../../../out-test/playwright/SharedRendererTests';
import { ITestContext, createTestContext, openTerminal } from '../../../out-test/playwright/TestUtils';
import { platform } from 'os';

Expand All @@ -29,4 +29,5 @@ test.describe('WebGL Renderer Integration Tests', async () => {
}

injectSharedRendererTests(ctxWrapper);
injectSharedRendererTestsStandalone(ctxWrapper);
});
18 changes: 18 additions & 0 deletions src/browser/renderer/dom/DomRendererRowFactory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,24 @@ describe('DomRendererRowFactory', () => {
}
});

it('should not display cursor for before initializing', () => {
const coreService = new MockCoreService();
coreService.isCursorInitialized = false;
const rowFactory = new DomRendererRowFactory(
dom.window.document,
new MockCharacterJoinerService(),
new MockOptionsService(),
new MockCoreBrowserService(),
coreService,
new MockDecorationService(),
new MockThemeService()
);
const spans = rowFactory.createRow(lineData, 0, true, 'block', undefined, 0, false, 5, EMPTY_WIDTH, -1, -1);
assert.equal(extractHtml(spans),
`<span> </span>`
);
});

describe('attributes', () => {
it('should add class for bold', () => {
const cell = CellData.fromCharData([0, 'a', 1, 'a'.charCodeAt(0)]);
Expand Down
2 changes: 1 addition & 1 deletion src/browser/renderer/dom/DomRendererRowFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export class DomRendererRowFactory {
}
}

if (!this._coreService.isCursorHidden && isCursorCell) {
if (!this._coreService.isCursorHidden && isCursorCell && this._coreService.isCursorInitialized) {
classes.push(RowCss.CURSOR_CLASS);
if (this._coreBrowserService.isFocused) {
if (cursorBlink) {
Expand Down
2 changes: 1 addition & 1 deletion src/common/TestUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class MockCharsetService implements ICharsetService {

export class MockCoreService implements ICoreService {
public serviceBrand: any;
public isCursorInitialized: boolean = false;
public isCursorInitialized: boolean = true;
public isCursorHidden: boolean = false;
public isFocused: boolean = false;
public modes: IModes = {
Expand Down
4 changes: 2 additions & 2 deletions test/playwright/Renderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { test } from '@playwright/test';
import { ITestContext, createTestContext, openTerminal } from './TestUtils';
import { ISharedRendererTestContext, injectSharedRendererTests } from './SharedRendererTests';
import { ISharedRendererTestContext, injectSharedRendererTestsStandalone, injectSharedRendererTests } from './SharedRendererTests';

let ctx: ITestContext;
const ctxWrapper: ISharedRendererTestContext = { value: undefined } as any;
Expand All @@ -18,5 +18,5 @@ test.afterAll(async () => await ctx.page.close());

test.describe('DOM Renderer Integration Tests', () => {
injectSharedRendererTests(ctxWrapper);
injectSharedRendererTestsStandalone(ctxWrapper);
});

36 changes: 35 additions & 1 deletion test/playwright/SharedRendererTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { IImage32, decodePng } from '@lunapaint/png-codec';
import { LocatorScreenshotOptions, test } from '@playwright/test';
import { ITheme } from 'xterm';
import { ITestContext, MaybeAsync, pollFor, pollForApproximate } from './TestUtils';
import { ITestContext, MaybeAsync, openTerminal, pollFor, pollForApproximate } from './TestUtils';

export interface ISharedRendererTestContext {
value: ITestContext;
Expand Down Expand Up @@ -1150,6 +1150,40 @@ export function injectSharedRendererTests(ctx: ISharedRendererTestContext): void
});
}

/**
* Injects shared renderer tests where it's required to re-initialize the terminal for each test.
* This is much slower than just calling `Terminal.reset` but testing some features needs this
* treatment.
*/
export function injectSharedRendererTestsStandalone(ctx: ISharedRendererTestContext): void {
test.beforeEach(async () => {
// Recreate terminal
await openTerminal(ctx.value);
ctx.value.page.evaluate(`
window.term.options.minimumContrastRatio = 1;
window.term.options.allowTransparency = false;
window.term.options.theme = undefined;
`);
// Clear the cached screenshot before each test
frameDetails = undefined;
});
test.describe('regression tests', () => {
test('#4790: cursor should not be displayed before focusing', async () => {
const theme: ITheme = {
cursor: '#0000FF'
};
await ctx.value.page.evaluate(`window.term.options.theme = ${JSON.stringify(theme)};`);
await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 255]);
await ctx.value.proxy.focus();
frameDetails = undefined;
await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 255, 255]);
await ctx.value.proxy.blur();
frameDetails = undefined;
await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 255]);
});
});
}

/**
* Gets the color of the pixel in the center of a cell.
* @param ctx The test context.
Expand Down

0 comments on commit 1618092

Please sign in to comment.