feat: report new URL after actions that trigger navigation#1853
feat: report new URL after actions that trigger navigation#1853rogeroliveira84 wants to merge 1 commit intoChromeDevTools:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Click, fill, press_key, drag, hover, type_text, and evaluate_script
already wait for cross-document navigations triggered by the action,
but the resulting URL was never surfaced to the agent. Callers had to
follow up with list_pages to figure out where they landed.
Propagate a {navigated: boolean} result out of WaitForHelper up through
McpPage so each handler can append a "Page navigated to <url>." line
when the action actually caused a navigation. Same-document (history
API) navigations are still filtered out by waitForNavigationStarted,
matching existing behavior.
Fixes ChromeDevTools#243
rogeroliveira84
force-pushed
the
feat/report-navigation-url
branch
from
5a3832c to
d281454
Compare
Lightning00Blade
left a comment
Lightning00Blade
left a comment
There was a problem hiding this comment.
Thanks for the PR,
there are some comments that would need to be address before we can merge.
| this.#abortController.abort(); | ||
| } | ||
|
|
||
| return {navigated}; |
There was a problem hiding this comment.
I think we can simplify this can just provide the URL here if it was changed.
| return {navigated}; | |
| return { | |
| ...(navigated ? {url: this.#page.url()} : {}) | |
| }; |
There was a problem hiding this comment.
Actually let's call it navigatedToUrl to be more explicit.
| * Whether a cross-document navigation started and finished during the | ||
| * action. Same-document (history API) navigations are not reported. | ||
| */ | ||
| navigated: boolean; |
There was a problem hiding this comment.
With the above change this will become
| navigated: boolean; | |
| navigatedToUrl?: string; |
| export interface WaitForEventsResult { | ||
| /** | ||
| * Whether a cross-document navigation started and finished during the | ||
| * action. Same-document (history API) navigations are not reported. |
There was a problem hiding this comment.
This is wrong we use Puppeteer waitForNavigation and that does support same-document navigation
https://pptr.dev/api/puppeteer.page.waitfornavigation
| ); | ||
| } | ||
|
|
||
| function appendNavigationIfAny( |
There was a problem hiding this comment.
Let's rename it to appendWaitForResult to match the interface we provide.
We can also move this into the WaitForHelper file as we can use in other places as well.
| const result = await selectedPage.waitForEventsAfterAction(async () => { | ||
| await performEvaluation(worker, fnString, [], response); | ||
| }); | ||
| if (result.navigated) { |
There was a problem hiding this comment.
This should use the helper.
| const result = await mcpPage.waitForEventsAfterAction(async () => { | ||
| await performEvaluation(evaluatable, fnString, args, response); | ||
| }); | ||
| if (result.navigated) { |
2 participants
Summary
click,fill,press_key,hover,drag,type_text,fill_form,click_at) andevaluate_scriptnow append aPage navigated to <url>.line to the response when the action triggers a cross-document navigation.WaitForHelper.waitForEventsAfterActionreturns{navigated: boolean}instead ofvoid, surfacing the navigation signal that was already being detected internally.navigate_pageornew_pagesince they already report the URL explicitly.Fixes #243
Why
Today, if a
clickcauses a page navigation, the response says "Successfully clicked on the element" with no indication that the page URL changed. The agent has to make an extralist_pagescall to discover where it landed. This saves that round-trip for every navigation-triggering action.Design
The existing
waitForNavigationStartedinWaitForHelperalready knows whether a cross-document navigation started. We propagate that signal as{navigated: boolean}through the return value ofwaitForEventsAfterAction→McpPage→ContextPageinterface, and let each handler append the URL line whennavigatedis true.Same-document (history API) navigations remain filtered out by the existing
waitForNavigationStartedlogic, matching current behavior. Click-opens-new-tab is a separate concern (#367).Test plan
Page navigated to <url>.