Skip to content

[REGRESSION]: Event Listeners not being removed if same handler is used for different events #15557

@Adam-Paterson1

Description

@Adam-Paterson1

Context:

  • GOOD Playwright Version: 1.22.2
  • BAD Playwright Version: 1.23.2
  • Operating System: Windows

Code Snippet
The original code that had the issue was a variant of the Network Settled Helper
This is a simplified version:

import { test, expect } from '@playwright/test';

test('should only have 1 handler', async ({ page }) => {
  let handlerCalled = 0;
  function requestHandler() {
    handlerCalled += 1;
  }

  page.on('requestfinished', requestHandler);
  page.on('requestfailed', requestHandler);

  page.removeListener('requestfinished', requestHandler);
  page.removeListener('requestfailed', requestHandler);

  page.on('requestfinished', requestHandler);
  page.on('requestfailed', requestHandler);

  await page.goto('https://playwright.dev/');
  const title = page.locator('.navbar__inner .navbar__title');
  await expect(title).toHaveText('Playwright');

  console.log(handlerCalled);
  // Playwright 1.23.2 produces 74
  // Playwright 1.22.2 produce 37
});

Describe the bug
The event listeners are not being removed when removeListener is called.

I had a quick look and I think the issue is in the _wrap method of joiningEventEmitter
It is mutating the listener passed in, by storing the wrapped function in the symbol wrapperListener.
When passing the same function to 2 different event handlers, they both get wrapped, with the second wrapping overriding the first.
At that point when we call removeListener, the wrapped method does not match the handler that was stored for the first wrapping, so that handler never gets removed.

Not sure if the above makes sense / is the cause of the issue, but it looked like the most likely cause to me.

Workaround
The issue can be worked around by copying the requestHandler to ensure that different functions are passed to listen to the different events.

import { test, expect } from '@playwright/test';

test('should only have 1 handler', async ({ page }) => {
  let handlerCalled = 0;
  function requestHandler() {
    handlerCalled += 1;
  }
  function requestHandler2() {
    handlerCalled += 1;
  }

  page.on('requestfinished', requestHandler);
  page.on('requestfailed', requestHandler2);

  page.removeListener('requestfinished', requestHandler);
  page.removeListener('requestfailed', requestHandler2);

  page.on('requestfinished', requestHandler);
  page.on('requestfailed', requestHandler2);

  await page.goto('https://playwright.dev/');
  const title = page.locator('.navbar__inner .navbar__title');
  await expect(title).toHaveText('Playwright');

  console.log(handlerCalled);
  // Both versions produce 37
});

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions