-
Notifications
You must be signed in to change notification settings - Fork 443
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
Issue-174 - createEnvironment in createHTMLDocument #177
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@adamdbradley would you be able to review this? I've added a test which breaks under current code and is resolved with the patch. |
Looks good, but would you also be able to add to the tests that replicates this issue? #174 (comment) Thanks |
@@ -92,7 +92,7 @@ test('document', async ({ page }) => { | |||
await expect(testCreateElementError_).toHaveText('no error'); | |||
|
|||
const testCreateHTMLDocument = page.locator('#testCreateHTMLDocument'); | |||
await expect(testCreateHTMLDocument).toHaveText('88mph hidden'); | |||
await expect(testCreateHTMLDocument).toHaveText('88mph hidden object'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamdbradley this is where I added the test from #174 Comment
const addlElm = doc.createElement('base'); | ||
const addlElmType = typeof addlElm; | ||
|
||
doc.body.textContent = '88mph ' + doc.visibilityState + ' ' + addlElmType; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamdbradley this is where I added the test from #174 Comment
@adamdbradley I added the test case, to the existing |
Thanks for the PR. I'll take a look into adding another test too. |
This seems to resolve Issue #174 by ensuring the window object created in
WorkerDocument.implementation.createHTMLDocument
is registered with theenvironments
so that subsequent calls tocreateElement
on the document it returns are properly found bygetOrCreateNodeInstance
.I've also updated the unit tests. Note these tests fail on the current
main
branch.