Skip to content
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

[Bug]: addLocatorHandler blocks #30424

Closed
ruifigueira opened this issue Apr 18, 2024 · 5 comments · Fixed by #30494
Closed

[Bug]: addLocatorHandler blocks #30424

ruifigueira opened this issue Apr 18, 2024 · 5 comments · Fixed by #30494
Assignees
Labels

Comments

@ruifigueira
Copy link
Contributor

Version

1.43.1

Steps to reproduce

Run the following test:

test("play wordle", async ({ page }) => {
    page.addLocatorHandler(page.getByRole('heading', { name: 'How To Play' }), async () => {
        await page.getByLabel('Close').click();
    });

    await page.goto('https://www.nytimes.com/games/wordle/index.html');
    await page.getByTestId('Play').click();
    for (const letter of 'salet')
        await page.locator(`[data-key=${letter}]`).click();
});

Expected behavior

It should close the help dialog and type SALET

Actual behavior

It closes the help dialog and blocks, not typing any letter. I'm attaching the captured trace:

trace.zip

Additional context

No response

Environment

System:
    OS: Windows 11 10.0.22631
    CPU: (8) x64 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
    Memory: 11.32 GB / 31.74 GB
  Binaries:
    Node: 18.16.0 - ~\scoop\apps\nvm\current\nodejs\nodejs\node.EXE
    npm: 9.7.1 - ~\scoop\apps\nvm\current\nodejs\nodejs\npm.CMD
    pnpm: 8.15.4 - ~\AppData\Local\pnpm\pnpm.EXE
  IDEs:
    VSCode: 1.88.1 - C:\Users\rui.figueira\scoop\apps\vscode\current\bin\code.CMD
  npmPackages:
    @playwright/test: ^1.43.1 => 1.43.1
@ruifigueira ruifigueira changed the title [Bug]: [Bug]: addLocatorHandler blocks Apr 18, 2024
@ruifigueira
Copy link
Contributor Author

ruifigueira commented Apr 18, 2024

I missed an await. New version of the test:

test('play wordle', async ({ page }) => {
    await page.addLocatorHandler(page.getByRole('heading', { name: 'How To Play' }), async () => {
        await page.getByLabel('Close').click();
    });

    await page.goto('https://www.nytimes.com/games/wordle/index.html');
    await page.getByTestId('Play').click();
    for (const letter of 'salet')
        await page.locator(`[data-key=${letter}]`).click();
});

And it's still failing. Trace:
trace_v2.zip

@ruifigueira
Copy link
Contributor Author

ruifigueira commented Apr 19, 2024

Ok, I figured it out. The callback was being called twice because clicking the Close button was not closing the dialog immediately.

To fix it, I have to wait for the locator to disappear:

test('play wordle', async ({ page }) => {
  await page.addLocatorHandler(page.getByRole('heading', { name: 'How To Play' }), async () => {
    await page.getByLabel('Close').click();
    await page.getByRole('heading', { name: 'How To Play' }).waitFor({ state: 'hidden' });
  });

  await page.goto('https://www.nytimes.com/games/wordle/index.html');
  await page.getByTestId('Play').click();
  for (const letter of 'salet')
    await page.locator(`[data-key=${letter}]`).click();
});

This pattern happens a lot due to fadeout animations. Maybe it would be worth to add a paragraph explaining this in the documentation.

@yury-s yury-s added the v1.44 label Apr 19, 2024
dgozman added a commit that referenced this issue Apr 24, 2024
- Automatically waiting for the overlay locator to be hidden, with
`allowStayingVisible` opt-out.
- `times: 1` option.
- `removeLocatorHandler(locator, handler)` method.
- Passing `locator` as first argument to `handler`.

Fixes #30471. Fixes #30424. Fixes #29779.
@cyrus-afshari
Copy link

Ok, I figured it out. The callback was being called twice because clicking the Close button was not closing the dialog immediately.

To fix it, I have to wait for the locator to disappear:

test('play wordle', async ({ page }) => {
  await page.addLocatorHandler(page.getByRole('heading', { name: 'How To Play' }), async () => {
    await page.getByLabel('Close').click();
    await page.getByRole('heading', { name: 'How To Play' }).waitFor({ state: 'hidden' });
  });

  await page.goto('https://www.nytimes.com/games/wordle/index.html');
  await page.getByTestId('Play').click();
  for (const letter of 'salet')
    await page.locator(`[data-key=${letter}]`).click();
});

This pattern happens a lot due to fadeout animations. Maybe it would be worth to add a paragraph explaining this in the documentation.

You spared me a lot of time and frustration! Thanks for sharing this, I was having the same issue and adding the waitFor({ state: 'hidden' }) solved it for me. Playwright's documentation on addLocatorHandler() should include your suggestion!

@dgozman
Copy link
Contributor

dgozman commented Jun 3, 2024

@cyrus-afshari Implicit waitFor at the end is now the default behavior of addLocatorHandler(). If that does not work for you, could you please file a new issue by filling in the "Bug Report" template and provide a repro?

@cyrus-afshari
Copy link

@cyrus-afshari Implicit waitFor at the end is now the default behavior of addLocatorHandler(). If that does not work for you, could you please file a new issue by filling in the "Bug Report" template and provide a repro?

I am on version 1.43.1, so I believe I don't have that new default behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants