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.

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

Fix: Ensure goto Uses Overridden Page in act/performPlaywrightMethod for Proper DOM Injection #453

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

askadityapandey
Copy link

Why

fixes #443
The current implementation of the "goto" command within the act handler calls page.goto on the original page instance. This prevents Stagehand’s DOM scripts from being injected as intended. This PR addresses the issue by explicitly handling "goto" commands to use the overridden page instance, ensuring that the DOM modifications occur correctly.


What Changed

  • Dedicated Branch for "goto" Command:
    Added a specific branch in the _performPlaywrightMethod method to handle the "goto" command. The URL is extracted from the command arguments and passed directly to this.stagehandPage.page.goto(url), ensuring that the overridden page (with the injected DOM scripts) is used.

  • Enhanced Logging and Error Handling:
    Introduced additional logging for the "goto" command execution to improve debugging and traceability. Error handling has been updated to catch and log exceptions appropriately during navigation.

  • DOM Settling After Navigation:
    After navigation, the method waits for the DOM to settle using _waitForSettledDom, guaranteeing that the page is fully loaded and the Stagehand scripts have been injected.


Test Plan

  1. Basic Navigation Test:

    • Execute a command like "goto https://example.com" via Stagehand.
    • Confirm that the page navigates to the specified URL.
    • Verify that the Stagehand DOM scripts are injected correctly.
  2. Error Handling:

    • Test with invalid URLs or simulate network errors.
    • Check that the error is logged appropriately and that a PlaywrightCommandException is thrown.
  3. Regression Test for Other Methods:

    • Run existing tests for other actions (e.g., "click", "fill", "press") to ensure their functionality remains unchanged.
  4. End-to-End Flow:

    • Execute a full action sequence that includes navigation along with other actions.
    • Verify that the overridden page instance is used consistently throughout the sequence and that the DOM modifications persist.

Copy link

changeset-bot bot commented Feb 1, 2025

⚠️ No Changeset found

Latest commit: 05499e5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

goto should be handled better in act/performPlaywrightMethod
1 participant