feat: report navigated URL in action responses#1954
feat: report navigated URL in action responses#1954masamaru0513 wants to merge 1 commit intoChromeDevTools:mainfrom
Conversation
masamaru0513
force-pushed
the
feat/report-navigated-url
branch
from
d58c34c to
ccf84a8
Compare
Report the new page URL when an action (click, fill, press_key, etc.) triggers a navigation. The URL is detected by comparing the page URL before and after waitForEventsAfterAction. Fixes ChromeDevTools#243
masamaru0513
force-pushed
the
feat/report-navigated-url
branch
from
ccf84a8 to
ca993a0
Compare
| request.params.timeout, | ||
| ); | ||
|
|
||
| response.appendResponseLine( |
There was a problem hiding this comment.
new_page already returns a list of open pages, no need to add anything
| const finalUrl = page.pptrPage.url(); | ||
| response.appendResponseLine( | ||
| `Successfully navigated to ${request.params.url}.`, | ||
| `Successfully navigated to ${finalUrl}.`, |
There was a problem hiding this comment.
For redirects (e.g. from foo to bar), the tool currently returns: "Successfully navigated to foo.
Pages:
1: bar"
I think this is already working as intended as it gives more info than just reporting the final URL twice.
| return helper.waitForEventsAfterAction(action, options); | ||
| await helper.waitForEventsAfterAction(action, options); | ||
| const urlAfter = this.pptrPage.url(); | ||
| if (urlAfter !== urlBefore) { |
There was a problem hiding this comment.
nit: Could be shortened to
return (urlAfter === urlBefore) ? {} : {navigatedToUrl: urlAfter};
| ); | ||
| assert.ok( | ||
| response.responseLines[1]?.includes('/nav-target'), | ||
| `Expected URL to contain /nav-target but got: ${response.responseLines[1]}`, |
There was a problem hiding this comment.
Instead of comparing the line in 2 (imprecise) parts, could we construct the whole response line (with server.getRoute) and check for equality?
|
Thanks! This mostly looks good to me, added a few comments. |
2 participants
Summary
Report the new page URL when an action triggers a navigation, so there is no need to re-query the pages list.
Fixes #243
Changes
McpPage.waitForEventsAfterActionnow compares the page URL before and after the action, returning{navigatedToUrl}when a navigation occurredappendNavigatedToUrlhelper inWaitForHelper.tsfor consistent URL reporting across toolsclick,click_at,hover,fill,fill_form,type_text,drag,press_key) use the helper to appendNavigated to <URL>when a navigation is detectedevaluate_scriptalso reports navigated URLs using the same helpernavigate_page(urltype) now reports the final URL after redirects instead of the requested URLnew_pagenow reports the navigated URL in the response messageOutput example
Before:
After:
Testing
navigate_pagereports final URLclickon a link → reports navigated URL ✅press_keyEnter on a focused link → reports navigated URL ✅evaluate_scriptwithwindow.location.hrefchange → reports navigated URL ✅evaluate_scriptwithout navigation → no "Navigated to" ✅navigate_pagereports final URL (with trailing slash after redirect) ✅navigate_pageback/forward → existing behavior preserved ✅