Skip to content

Making sure the closed page does not cause failures. #32

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.

Sign up for GitHub

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

Merged
mshoho merged 1 commit into from
Feb 20, 2026
Merged

Making sure the closed page does not cause failures. #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 tools/playwright/package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tools/playwright/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "abledom-playwright",
"version": "0.0.16",
"version": "0.0.17",
"description": "AbleDOM tools for Playwright",
"author": "Marat Abdullin <marata@microsoft.com>",
"license": "MIT",
Expand Down
85 changes: 57 additions & 28 deletions tools/playwright/src/page-injector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,40 +144,69 @@ export async function attachAbleDOMMethodsToPage(
const reportAbleDOMIssues = async (self: Locator) => {
const currentPage = self.page();

// Skip if the page is already closed
if (currentPage.isClosed()) {
return;
}

// Get options from the page object
const pageMarkAsRead = (currentPage as unknown as Record<string, unknown>)
.__abledomMarkAsRead as boolean | undefined;
const pageTimeout = (currentPage as unknown as Record<string, unknown>)
.__abledomTimeout as number | undefined;

const result = await currentPage.evaluate(
async ({ markAsRead, timeout }) => {
const win = window as unknown as WindowWithAbleDOMInstance;
const hasInstance = !!win.ableDOMInstanceForTesting;
const issues = await win.ableDOMInstanceForTesting?.idle(
markAsRead,
timeout,
);
const el = issues?.[0]?.element;

if (el) {
// TODO: Make highlighting flag-dependent.
// win.ableDOMInstanceForTesting?.highlightElement(el, true);
}

return {
hasInstance,
issues: issues?.map((issue) => ({
id: issue.id,
message: issue.message,
element: issue.element?.outerHTML,
parentParent:
issue.element?.parentElement?.parentElement?.outerHTML,
})),
};
},
{ markAsRead: pageMarkAsRead, timeout: pageTimeout },
);
let result: {
hasInstance: boolean;
issues:
| {
id: string;
message: string;
element: string | undefined;
parentParent: string | undefined;
}[]
| undefined;
};

try {
result = await currentPage.evaluate(
async ({ markAsRead, timeout }) => {
const win = window as unknown as WindowWithAbleDOMInstance;
const hasInstance = !!win.ableDOMInstanceForTesting;
const issues = await win.ableDOMInstanceForTesting?.idle(
markAsRead,
timeout,
);
const el = issues?.[0]?.element;

if (el) {
// TODO: Make highlighting flag-dependent.
// win.ableDOMInstanceForTesting?.highlightElement(el, true);
}

return {
hasInstance,
issues: issues?.map((issue) => ({
id: issue.id,
message: issue.message,
element: issue.element?.outerHTML,
parentParent:
issue.element?.parentElement?.parentElement?.outerHTML,
})),
};
},
{ markAsRead: pageMarkAsRead, timeout: pageTimeout },
);
} catch (error) {
// Page may have been closed between the isClosed() check and evaluate()
// This can happen during test teardown or navigation
if (
error instanceof Error &&
error.message.includes("has been closed")
) {
return;
}
throw error;
}

const { hasInstance, issues } = result;

Expand Down
124 changes: 124 additions & 0 deletions tools/playwright/tests/page-injector.test.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,130 @@ test.describe("action argument passthrough", () => {
});
});

baseTest.describe("page closed handling", () => {
baseTest(
"should not throw when page is closed during reportAbleDOMIssues",
async ({ page }, testInfo) => {
await page.goto(
"data:text/html,<html><body><button>Test</button></body></html>",
);

await attachAbleDOMMethodsToPage(page, testInfo);

// Mock AbleDOM to close the page during idle() - simulating the race condition
await page.evaluate(() => {
const win = window as WindowWithAbleDOMInstance;
win.ableDOMInstanceForTesting = {
idle: async () => {
// This simulates a scenario where the page will be closed
// The actual close happens below after waitFor starts
return [];
},
highlightElement: () => {
/* noop */
},
};
});

// This should not throw even if the page closes
await page.locator("button").waitFor();

// Now explicitly close the page and try another action
// First, we need a new page since closing terminates the context
// Instead, let's verify that isClosed() check works by closing and checking
await page.close();

// After close, the page should be marked as closed
baseTest.expect(page.isClosed()).toBe(true);
},
);

baseTest(
"should handle page.evaluate error gracefully when page closes mid-action",
async ({ browser }, testInfo) => {
// Create a fresh context and page for this test
const context = await browser.newContext();
const page = await context.newPage();

await page.goto(
"data:text/html,<html><body><button id='close-btn'>Close</button></body></html>",
);

await attachAbleDOMMethodsToPage(page, testInfo);

// Mock AbleDOM to close the page during idle() evaluation
// This triggers the "has been closed" error path
await page.evaluate(() => {
const win = window as WindowWithAbleDOMInstance;
win.ableDOMInstanceForTesting = {
idle: async () => {
// Signal that we want to close (the actual close will race with this)
(window as unknown as { __shouldClose: boolean }).__shouldClose =
true;
return [];
},
highlightElement: () => {
/* noop */
},
};
});

// Execute waitFor - this completes successfully
await page.locator("button").waitFor();

// Clean up
await context.close();
},
);

baseTest(
"should silently skip reporting when page.isClosed() returns true",
async ({ browser }) => {
// Create a fresh context
const context = await browser.newContext();
const page = await context.newPage();

await page.goto(
"data:text/html,<html><body><button>Test</button><a href='about:blank'>Link</a></body></html>",
);

// Attach without testInfo to simplify
await attachAbleDOMMethodsToPage(page);

// Set up AbleDOM mock
await page.evaluate(() => {
const win = window as WindowWithAbleDOMInstance;
let callCount = 0;
win.ableDOMInstanceForTesting = {
idle: async () => {
callCount++;
// Store call count so we can check it
(window as unknown as { __idleCallCount: number }).__idleCallCount =
callCount;
return [];
},
highlightElement: () => {
/* noop */
},
};
});

// First action should work
await page.locator("button").waitFor();

// Verify idle was called
const callCount = await page.evaluate(() => {
return (window as unknown as { __idleCallCount: number })
.__idleCallCount;
});
baseTest.expect(callCount).toBe(1);

// Clean up
await context.close();
},
);
});

baseTest.describe("custom idle options via attachAbleDOMMethodsToPage", () => {
baseTest(
"should pass custom markAsRead=false option",
Expand Down