stagehand icon indicating copy to clipboard operation
stagehand copied to clipboard

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

Open askadityapandey opened this issue 11 months ago • 2 comments

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.

askadityapandey avatar Feb 01 '25 14:02 askadityapandey

⚠️ No Changeset found

Latest commit: 05499e52bc392e979ef428252a489e3f9cf0dc6e

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

changeset-bot[bot] avatar Feb 01 '25 14:02 changeset-bot[bot]

my bad, didn't see this until now! thank you so much for your contribution, will look at this in depth probably later this week or this weekend. sincerely appreciate your hard work and detailed write up

kamath avatar Feb 05 '25 22:02 kamath