microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
64.95k stars 3.53k forks source link

Window scope is different for webkit #749

Closed jperl closed 4 years ago

jperl commented 4 years ago

In the example below, I use window._clickHandlerIsSetup to make sure the code is only run once. This works for chrome and firefox but not webkit. If you run this you will see on webkit the document.addEventListener gets injected twice, because numReceived === 2.

It would be nice to have some way to prevent evaluateOnNewDocument code from executing multiple times in the same context, even if using a window flag is not possible.

const assert = require("assert");
const playwright = require("playwright");

const test = async type => {
  const browser = await playwright[type].launch({ headless: false });

  const context = await browser.newContext();

  const page = await context.newPage();

  let numReceived = 0;

  await page.exposeFunction("clickCallback", () => {
    numReceived += 1;
  });

  await page.evaluateOnNewDocument(() => {
    if (!window._clickHandlerIsSetup) {
      window._clickHandlerIsSetup = true;

      document.addEventListener("click", () => clickCallback(), {
        capture: true,
        passive: true
      });
    }
  });

  await page.goto("http://google.com");

  const handle = await page.$('zs="About"');
  await handle.click();
  await page.waitForNavigation();

  assert.strictEqual(
    numReceived,
    1,
    `incorrect number of callbacks for ${type}`
  );

  await browser.close();
};

test("chromium");
test("firefox");
test("webkit");
jperl commented 4 years ago

For headless, both chromium and firefox work. Interestingly webkit has a different error for headless. It cannot find the "About" element.

Screen Shot 2020-01-29 at 12 53 48 PM
JoelEinbinder commented 4 years ago

@yury-s any thoughts? It looks like we are running evaluateOnNewDocument twice for the same document. But the window object has been cleared in the meantime? Confusing!

yury-s commented 4 years ago

It may well be that bootstrap scrips are evaluated more than once in webkit (because of a bug). We are addressing some of that with the next WebKit roll (https://github.com/microsoft/playwright/pull/737), I'll try to reproduce it after that change is committed.

pavelfeldman commented 4 years ago

I don't think we have any bugs in the webkit worlds, the test coverage is solid there. Could About be dynamic? You should always use

const handle = await page.click('zs="About"');

that waits for selector element to be available / visible instead of fetching the handle and then using it.

jperl commented 4 years ago

My main issue is the headful example above: not being able to prevent evaluateOnNewDocument from running twice for the same document. The example above shows different behavior from webkit vs chrome/firefox.

Re: the headless issue, when I changed the code to await page.click('zs="About"'); it fails with (node:4215) UnhandledPromiseRejectionWarning: TimeoutError: waiting for selector "[visible] zs="About"" failed: timeout 30000ms exceeded.

pavelfeldman commented 4 years ago

Ok, can repro, should be fixed with https://github.com/microsoft/playwright/pull/736 when new webkit bakes.

pavelfeldman commented 4 years ago

This should work now. Please tell us if it does not!

I'd still recommend using the following pattern for the click that causes navigation:

  const page = await context.newPage('https://www.google.com');
  await Promise.all([
    page.waitForNavigation(),
    page.click('zs="About"')
  ]);

That way if navigation completes before click is handled (click on a local #anchor), we don't get stuck...

jperl commented 4 years ago

Both headless and headful work with the latest master. Thanks for the quick fix 🙏and the suggestion about navigation completing before the click 👍.