feat: wait for pending navigation to resolve before many actions

[dgozman]

This includes all actions that perform locator handler check.

Note this makes it impossible to interact with the page while a main frame navigation is ongoing. This was already the case for Chromium, but now WebKit and Firefox align with it.

Setting PLAYWRIGHT_SKIP_NAVIGATION_CHECK environment variable disables this behavior.

[github-actions]

Test results for "tests 1"

2 flaky

⚠️ [chromium-library] › library/popup.spec.ts:264:3 › should not throw when click closes popup
⚠️ [webkit-library] › library/video.spec.ts:797:5 › screencast › should work with video+trace

35759 passed, 680 skipped
✔️✔️✔️

Merge workflow run.

[chrisbottin]

Hey @dgozman

We have noticed that some of our tests stopped working after bumping our Playwright version from 1.48.2 to 1.49.0.

I debugged the issue and I pinned it down to the changes in this pull request.

The problem we are having is when we click on a link which triggers the beforeunload dialog, after we dismiss the dialog and try to interact with the page again, we get timeout errors.

Here is a simple test replicating the error ...

const {chromium} = require('playwright');
const {expect} = require('@playwright/test');

(async () => {
    const browser = await chromium.launch();
    const page = await browser.newPage();

    await page.mainFrame().goto('https://www.npmjs.com/');

    expect(await page.mainFrame().title()).toStrictEqual('npm | Home');

    await page.evaluate(() => {
        window.onbeforeunload = () => false;
    });

    page.on('dialog', async dialog => {
        await dialog.dismiss();
    });

    await page.getByRole('menuitem', {name: 'Teams'}).click({noWaitAfter: true});

    await page.evaluate(() => {
        window.onbeforeunload = null;
    });

    // Timeout error will occur here:
    await page.getByRole('menuitem', {name: 'Teams'}).click({timeout: 1000});

    expect(await page.mainFrame().title()).toStrictEqual('npm | Teams');

    await browser.close();
})();

The test fails on the line await page.getByRole('menuitem', {name: 'Teams'}).click({timeout: 1000}); with the error:

locator.click: Timeout 1000ms exceeded.
Call log:
  - waiting for getByRole('menuitem', { name: 'Teams' })
  - waiting for navigation to finish...

During debugging, I noticed the pendingDocument() is set to {documentId: undefined, request: undefined}

If this line

playwright/packages/playwright-core/src/server/page.ts

Line 472 in b5bd543

if (!mainFrame || !mainFrame.pendingDocument())

is changed to the following, our problem disappears:

if (!mainFrame || !mainFrame.pendingDocument() || !mainFrame.pendingDocument().documentId)
      return;

Is this a genuine issue that should be fixed?

[dgozman]

@chrisbottin This is a genuine issue, I filed #33806 on your behalf. We'll take a look.

[chrisbottin]

@dgozman Thanks, I also raised #33803 after posting my comment here.
I've closed it as duplicate now.