-
Notifications
You must be signed in to change notification settings - Fork 61
feat: don't close new opened tabs (#161) #169
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
Changes from 3 commits
78525a7
b2ee152
535339d
a474768
bbd8289
55800e4
29e34e2
c0959a2
c51f8d0
d96a372
15719dc
a274c02
6f35684
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"stagehand": patch | ||
--- | ||
|
||
Don't close new opened tabs | ||
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -40,7 +40,10 @@ async def inject_custom_scripts(self, pw_page: Page): | |||||
async def get_stagehand_page(self, pw_page: Page) -> StagehandPage: | ||||||
if pw_page not in self.page_map: | ||||||
return await self.create_stagehand_page(pw_page) | ||||||
return self.page_map[pw_page] | ||||||
stagehand_page = self.page_map[pw_page] | ||||||
# Update active page when getting a page | ||||||
# self.set_active_page(stagehand_page) | ||||||
miguelg719 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
# Update active page when getting a page | |
# self.set_active_page(stagehand_page) |
miguelg719 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
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.
lets discuss the event lifecycle here.
User clicks on a link which opens a new page - this method docstring says it happens synchronously in the event lifecycle, but the code suggest its an async side event.
we have no guarantee that stagehand operations will be continuing on the new page the moment a new page is requested in this code - or am I missing something? There could be a couple ms or hundreds of ms gap where new stagehand page has not been swapped to but stagehand commands continue to start firing - unless we make this a truly execution blokcing and not a side async event loop.
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.
we need to make it blocking/sync @filip-michalsky
This covers updates happening api-side, which are bound to race conditions anyways but blocking/awaiting will reduce its effect
Uh oh!
There was an error while loading. Please reload this page.