Skip to content

Support shadow dom in webgl renderer #5334

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions addons/addon-webgl/src/WebglRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export class WebglRenderer extends Disposable implements IRenderer {

[this._rectangleRenderer.value, this._glyphRenderer.value] = this._initializeWebGLState();

this._isAttached = this._coreBrowserService.window.document.body.contains(this._core.screenElement!);
this._isAttached = this._core.screenElement!.isConnected;

this._register(toDisposable(() => {
for (const l of this._renderLayers) {
Expand Down Expand Up @@ -322,7 +322,7 @@ export class WebglRenderer extends Disposable implements IRenderer {

public renderRows(start: number, end: number): void {
if (!this._isAttached) {
if (this._coreBrowserService.window.document.body.contains(this._core.screenElement!) && this._charSizeService.width && this._charSizeService.height) {
if (this._core.screenElement?.isConnected && this._charSizeService.width && this._charSizeService.height) {
this._updateDimensions();
this._refreshCharAtlas();
this._isAttached = true;
Expand Down
11 changes: 9 additions & 2 deletions test/playwright/SharedRendererTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1280,10 +1280,10 @@ enum CellColorPosition {
* treatment.
*/
export function injectSharedRendererTestsStandalone(ctx: ISharedRendererTestContext, setupCb: () => Promise<void> | void): void {
test.describe('standalone tests', () => {
const setupTests = ({ shadowDom }: { shadowDom: boolean }): void => {
test.beforeEach(async () => {
// Recreate terminal
await openTerminal(ctx.value);
await openTerminal(ctx.value, {}, { useShadowDom: shadowDom });
await ctx.value.page.evaluate(`
window.term.options.minimumContrastRatio = 1;
window.term.options.allowTransparency = false;
Expand All @@ -1308,6 +1308,13 @@ export function injectSharedRendererTestsStandalone(ctx: ISharedRendererTestCont
await pollFor(ctx.value.page, () => getCellColor(ctx.value, 1, 1), [0, 0, 0, 255]);
});
});
};

test.describe('standalone tests', () => {
setupTests({ shadowDom: false });
});
test.describe('standalone tests (Shadow dom)', () => {
setupTests({ shadowDom: true });
});
}

Expand Down
45 changes: 40 additions & 5 deletions test/playwright/TestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ class TerminalCoreProxy {
}
}

export async function openTerminal(ctx: ITestContext, options: ITerminalOptions | ITerminalInitOnlyOptions = {}, testOptions: { loadUnicodeGraphemesAddon: boolean } = { loadUnicodeGraphemesAddon: true }): Promise<void> {
export async function openTerminal(ctx: ITestContext, options: ITerminalOptions | ITerminalInitOnlyOptions = {}, { useShadowDom = false, loadUnicodeGraphemesAddon = true }: { loadUnicodeGraphemesAddon?: boolean, useShadowDom?: boolean } = { }): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you keep testOptions as it was?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I'm not sure how to be able to:

  • keep the loadUnicodeGraphemesAddon default value
  • have a default value on useShadowDom
  • make it work when only providing the useShadowDom parameter

Without adding some boilerplate in the function itself 🤔 what is the issue with that syntax?

await ctx.page.evaluate(`
if ('term' in window) {
try {
Expand All @@ -423,13 +423,48 @@ export async function openTerminal(ctx: ITestContext, options: ITerminalOptions
// HACK: Tests may have side effects that could cause the terminal not to be removed. This
// assertion catches this case early.
strictEqual(await ctx.page.evaluate(`document.querySelector('#terminal-container').children.length`), 0, 'there must be no terminals on the page');
await ctx.page.evaluate(`

let script = `
window.term = new window.Terminal(${JSON.stringify({ allowProposedApi: true, ...options })});
window.term.open(document.querySelector('#terminal-container'));
`);
let element = document.querySelector('#terminal-container');

// Remove shadow root if it exists
const newElement = element.cloneNode(false);
element.replaceWith(newElement);
element = newElement
Comment on lines +431 to +434
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this stuff only happen if !!useShadowDom?

Copy link
Author

@CGNonofr CGNonofr May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because if the previous test was using shadow dom, the next test need to clean it no matter what (just like the call to dispose on any existing term instance a few line above)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(if an element has a shadow dom attached, it doesn't seem we can still continue injecting other elements in it)

`;


if (useShadowDom) {
script += `
const shadowRoot = element.attachShadow({ mode: "open" });

// Copy parent styles to shadow DOM
const styles = Array.from(document.querySelectorAll('link[rel="stylesheet"]'));
styles.forEach((styleEl) => {
const clone = document.createElement('link');
clone.rel = 'stylesheet';
clone.href = styleEl.href;
shadowRoot.appendChild(clone);
});

// Create new element inside the shadow DOM
element = document.createElement('div');
element.style.width = '100%';
element.style.height = '100%';
shadowRoot.appendChild(element);
`;
}

script += `
window.term.open(element);
`;

await ctx.page.evaluate(script);

// HACK: This is a soft layer breaker that's temporarily included until unicode graphemes have
// more complete integration tests. See https://github.com/xtermjs/xterm.js/pull/4519#discussion_r1285234453
if (testOptions.loadUnicodeGraphemesAddon) {
if (loadUnicodeGraphemesAddon) {
await ctx.page.evaluate(`
window.unicode = new UnicodeGraphemesAddon();
window.term.loadAddon(window.unicode);
Expand Down