Skip to content

feat(elementhandle): add timeout and signal to inputValue#41548

Closed
Skn0tt wants to merge 1 commit into
microsoft:mainfrom
Skn0tt:inputvalue-timeout-signal
Closed

feat(elementhandle): add timeout and signal to inputValue#41548
Skn0tt wants to merge 1 commit into
microsoft:mainfrom
Skn0tt:inputvalue-timeout-signal

Conversation

@Skn0tt

@Skn0tt Skn0tt commented Jun 30, 2026

Copy link
Copy Markdown
Member

Adds timeout and signal to ElementHandle.inputValue, following on from #41141.

⚠️ Breaking change

inputValue will start timing out again.

The history is worth spelling out, because I got it wrong at first. inputValue has advertised a timeout option since v1.13, and until v1.53 it was honoured: the server ran it through the usual path and applied the context's default action timeout (30s). Then #35988 ("move timeout handling to the client") pinned all the ElementHandle getters - inputValue, textContent, getAttribute, isChecked and friends - to { timeout: 0 }, and the client inputValue() was never taught to send a timeout. So since v1.53 it's had no timeout at all, and the documented option has been a silent no-op.

This PR wires timeout back through the protocol properly and adds signal next to it. Because the client sends the default timeout again, inputValue will once more time out (30s by default) where for the last several releases it would wait forever. That restores the pre-1.53 behaviour, but it's a real change for anyone who came to rely on it never timing out - hence the loud warning.

The alternative would be to go the other way: accept that inputValue doesn't really need a timeout (it runs against an already-resolved element) and just deprecate the timeout option instead of reviving it. That keeps today's behaviour and quietly retires a dead option. I've gone with reviving it here because it lines inputValue back up with how it worked pre-1.53 and lets us add signal alongside, but I'm happy to flip to the deprecation route if you'd prefer it.

Why just inputValue? The same v1.53 regression hit every ElementHandle getter - they're all still pinned to timeout: 0 - but inputValue is the only one that ever advertised a timeout option in the docs and types, so it's the only one where the public API and the runtime actually disagree. The other getters (textContent, getAttribute, isChecked, ...) never promised a timeout, so I've left them alone.

Refs #40578

ElementHandle.inputValue advertised a `timeout` option since v1.13, but
the protocol entry in handles.yml declared no parameters, so the value
was silently dropped at the wire validator and never applied. This wires
it through properly and adds `signal` alongside it.

Note this is a behaviour change: inputValue now honours the default
timeout where before it effectively had none.

Refs: microsoft#40578
@Skn0tt Skn0tt assigned dgozman and unassigned dgozman Jun 30, 2026
@Skn0tt Skn0tt requested a review from dgozman June 30, 2026 14:06
@Skn0tt

Skn0tt commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

I think we should mark timeout as deprecated.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

5 failed
❌ [chromium] › mcp/annotate.spec.ts:230 › should capture annotations via show --annotate @mcp-macos-latest-chromium
❌ [chromium] › mcp/annotate.spec.ts:173 › user-initiated annotate downloads zip with feedback.md @mcp-windows-latest-chromium
❌ [chromium] › mcp/http.spec.ts:349 › client should receive list roots request @mcp-ubuntu-latest-chromium
❌ [firefox] › mcp/http.spec.ts:349 › client should receive list roots request @mcp-ubuntu-latest-firefox
❌ [webkit] › mcp/annotate.spec.ts:110 › should abort annotation when last screenshot is removed @mcp-ubuntu-latest-webkit

7456 passed, 1132 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

6 flaky ⚠️ [chromium-library] › library/video.spec.ts:113 › screencast › should capture static page `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:717 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/browsertype-connect.spec.ts:807 › launchServer › should upload a folder `@firefox-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-test-output.spec.ts:118 › should collapse repeated console messages for test `@windows-latest-node22`
⚠️ [playwright-test] › ui-mode-test-output.spec.ts:118 › should collapse repeated console messages for test `@ubuntu-latest-node22`

49215 passed, 1163 skipped


Merge workflow run.

@Skn0tt

Skn0tt commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

discussed, let's deprecate timeout here instead.

@Skn0tt Skn0tt closed this Jun 30, 2026
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.

2 participants