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.

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

Jump to bottom

[BUG] Error serializing page.evaluate return value if URL is overridden on the page #21109

Closed
simonychoy opened this issue Feb 22, 2023 · 2 comments · Fixed by #21112 or #21542
Closed

[BUG] Error serializing page.evaluate return value if URL is overridden on the page #21109

simonychoy opened this issue Feb 22, 2023 · 2 comments · Fixed by #21112 or #21542
Assignees
@yury-s
Labels
v1.32

Comments

@simonychoy
Copy link

Context:

  • Playwright Version: 1.31.0
  • Operating System: Mac
  • Node.js version: 14.20.0
  • Browser: All

Code Snippet

I created this repo to reproduce this issue, but the code to reproduce this is shown below:

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

const simpleHtmlDataUrl = `data:text/html,<html><script type="application/javascript">URL = 'this is a string value and not a URL';</script><body><h1>Test page</h1></body></html>`;

test("Evaluate script on page where URL is overridden", async ({ page }) => {
  await page.goto(simpleHtmlDataUrl);

  const data = { text: "some data", value: 1 };

  const result = await page.evaluate((data) => {
    const { text } = data;
    return { myTextData: text };
  }, data);
  expect(result).toEqual({ myTextData: "some data" });
});

Describe the bug

If a site overrides URL with a string value, attempting to evaluate a script on a page that returns an object will return undefined due to the isUrl check throwing an error during serialization. Perhaps the check should be similar to the isError check where the check is wrapped in a try/catch? (I know you shouldn’t override the URL property, but I can’t control the target website).

I am willing to open a PR to fix this if you'd like.
@yury-s yury-s self-assigned this Feb 22, 2023
@yury-s yury-s added the v1.32 label Feb 22, 2023
@yury-s yury-s mentioned this issue Feb 22, 2023
Merged
yury-s added a commit that referenced this issue Feb 22, 2023
@yury-s
fix: eval serialization w/ overridden URL/RegExp/Date (#21112)
d6dae8e 
Fixes #21109
@yury-s yury-s reopened this Feb 22, 2023
@yury-s yury-s mentioned this issue Feb 22, 2023
Merged
yury-s added a commit that referenced this issue Feb 23, 2023
@yury-s
test: mark new test as failing in ff (#21122)
2bb6bc7 
The test is failing for overridden Date in Firefox.

#21109
@simonychoy
Copy link
Author

Thank you @yury-s! Should I close this issue or leave it open? (I noticed you had reopened it)

@yury-s
Copy link
Member

yury-s commented Mar 1, 2023

I reopened it as the new test doesn't pass in Firefox, we want it work in all browsers.

@aslushnikov aslushnikov mentioned this issue Mar 9, 2023
Merged
aslushnikov added a commit to aslushnikov/playwright that referenced this issue Mar 9, 2023
@aslushnikov
feat(firefox): roll Firefox to 1387
3d469ee 
Fixes microsoft#21109
Fixes microsoft#21430
aslushnikov added a commit to aslushnikov/playwright that referenced this issue Mar 10, 2023
@aslushnikov
feat(firefox): roll Firefox to 1387
e0eb465 
Fixes microsoft#21109
Fixes microsoft#21430
aslushnikov added a commit to aslushnikov/playwright that referenced this issue Mar 10, 2023
@aslushnikov
feat(firefox): roll Firefox to 1387
72c381e 
Fixes microsoft#21109
Fixes microsoft#21430
aslushnikov added a commit that referenced this issue Mar 10, 2023
@aslushnikov
feat(firefox): roll Firefox to 1387 (#21542)
870edd1 
Fixes #21109
Fixes #21430
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yury-s yury-s

Labels
v1.32
Projects
None yet
Milestone
No milestone
2 participants
@yury-s @simonychoy