From 58c7fe05434f56c29c00056579fe6ce13c1b8d21 Mon Sep 17 00:00:00 2001 From: Andrey Mnatsakanov Date: Tue, 4 Aug 2026 11:02:05 +0200 Subject: [PATCH] fix(mcp): fill_form check/uncheck/select use the native setter and trusted events The check/uncheck/select branches of browser_fill_form were left on the pre-#530 pattern: a direct `el.checked = true` / `el.selectedIndex = i` plus a raw, untrusted `change` event. For a React-controlled checkbox the direct assignment runs the framework's value tracker in lockstep, so onChange never commits and the box reverts; and any handler gating on event.isTrusted ignores the untrusted events -- the exact bug class #530 fixed for text inputs two lines away. Route check/uncheck through __obscura_setFieldValue(el, 'checked', ...) so the write goes through the prototype setter and leaves the tracker stale, and dispatch input+change wrapped in __obscura_markTrusted for all three branches, matching the text-input branch. Fixes #550 --- crates/obscura-mcp/src/lib.rs | 85 ++++++++++++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 5 deletions(-) diff --git a/crates/obscura-mcp/src/lib.rs b/crates/obscura-mcp/src/lib.rs index 4937da4..4cf09ca 100644 --- a/crates/obscura-mcp/src/lib.rs +++ b/crates/obscura-mcp/src/lib.rs @@ -1298,15 +1298,17 @@ fn tool_fill_form(args: &Value, state: &mut BrowserState) -> Result format!(r#"(function(){{ var el = document.querySelector({sel}); if (!el) return "error:not found"; - el.checked = true; - el.dispatchEvent(new Event('change', {{bubbles:true}})); + globalThis.__obscura_setFieldValue(el, 'checked', true); + el.dispatchEvent(globalThis.__obscura_markTrusted(new Event('input', {{bubbles:true}}))); + el.dispatchEvent(globalThis.__obscura_markTrusted(new Event('change', {{bubbles:true}}))); return "ok"; }})()"#, sel = serde_json::to_string(&selector).unwrap()), "uncheck" => format!(r#"(function(){{ var el = document.querySelector({sel}); if (!el) return "error:not found"; - el.checked = false; - el.dispatchEvent(new Event('change', {{bubbles:true}})); + globalThis.__obscura_setFieldValue(el, 'checked', false); + el.dispatchEvent(globalThis.__obscura_markTrusted(new Event('input', {{bubbles:true}}))); + el.dispatchEvent(globalThis.__obscura_markTrusted(new Event('change', {{bubbles:true}}))); return "ok"; }})()"#, sel = serde_json::to_string(&selector).unwrap()), "select" => format!(r#"(function(){{ @@ -1323,7 +1325,8 @@ fn tool_fill_form(args: &Value, state: &mut BrowserState) -> Result format!(r#"(function(){{ @@ -1846,4 +1849,76 @@ mod tests { json!(r#"{"domValue":"form-filled","controlledState":"form-filled","controlledUpdates":3,"lastInputTarget":"field","lastInputTrusted":true}"#), ); } + + #[tokio::test(flavor = "current_thread")] + async fn fill_form_check_and_select_use_native_setter_and_trusted_events() { + let mut state = BrowserState::new(None, None, false); + state + .page_mut() + .navigate( + "data:text/html,
\ +
", + ) + .await + .expect("test page should navigate"); + + // Install a React-style tracker on the checkbox's `checked`, redefined on + // the instance. A direct `el.checked = true` runs this wrapper in lockstep, + // so a change handler comparing target.checked to the tracked value sees no + // change and never commits. Writing through the prototype setter (what + // __obscura_setFieldValue does) leaves the tracker stale so the edit + // registers. Also record whether the dispatched change is trusted. + state.page_mut().evaluate( + r#"(function () { + var box = document.getElementById('box'); + var root = document.getElementById('root'); + var d = Object.getOwnPropertyDescriptor(box.constructor.prototype, 'checked'); + var tracked = box.checked; + Object.defineProperty(box, 'checked', { + configurable: true, + get: function () { return d.get.call(this); }, + set: function (v) { tracked = !!v; d.set.call(this, v); } + }); + window.__checkedCommitted = false; + window.__checkTrusted = false; + window.__selectTrusted = false; + root.addEventListener('change', function (event) { + if (event.target.id === 'box') { + window.__checkTrusted = event.isTrusted; + if (event.target.checked !== tracked) { + tracked = event.target.checked; + window.__checkedCommitted = true; + } + } else if (event.target.id === 'sel') { + window.__selectTrusted = event.isTrusted; + } + }); + })()"#, + ); + + tool_fill_form( + &json!({ + "fields": [ + { "selector": "#box", "type": "check" }, + { "selector": "#sel", "type": "select", "value": "b" } + ] + }), + &mut state, + ) + .expect("browser_fill_form should succeed"); + + let actual = state.page_mut().evaluate( + r#"JSON.stringify({ + domChecked: document.getElementById('box').checked, + checkedCommitted: window.__checkedCommitted, + checkTrusted: window.__checkTrusted, + selValue: document.getElementById('sel').value, + selectTrusted: window.__selectTrusted + })"#, + ); + assert_eq!( + actual, + json!(r#"{"domChecked":true,"checkedCommitted":true,"checkTrusted":true,"selValue":"b","selectTrusted":true}"#), + ); + } }