fix: add disabled flag, external form fields, and param ordering

Address review feedback from @karlseguin:

1. Use Form.getElements() instead of manual TreeWalker for field
   collection. This reuses NodeLive(.form) which handles fields
   outside the <form> via the form="id" attribute per spec.

2. Add disabled detection: checks both the element's disabled
   attribute and ancestor <fieldset disabled> (with first-legend
   exemption per spec). Fields are flagged rather than excluded -
   agents need visibility into disabled state.

3. allocator is now the first parameter in collectForms/helpers.

4. handleDetectForms returns InvalidParams on bad input instead
   of silently swallowing parse errors.

5. Added tests for disabled fields, disabled fieldsets, and
   external form fields via form="id".

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

src/browser/forms.zig

@@ -46,6 +46,7 @@ pub const FormField = struct {
     name: ?[]const u8,
     input_type: ?[]const u8,
     required: bool,
+    disabled: bool,
     value: ?[]const u8,
     placeholder: ?[]const u8,
     options: []SelectOption,
@@ -76,6 +77,11 @@ pub const FormField = struct {
             try jw.write(true);
         }
 
+        if (self.disabled) {
+            try jw.objectField("disabled");
+            try jw.write(true);
+        }
+
         if (self.value) |v| {
             try jw.objectField("value");
             try jw.write(v);
@@ -136,6 +142,8 @@ pub const FormInfo = struct {
 };
 
 /// Collect all forms and their fields under `root`.
+/// Uses Form.getElements() to include fields outside the <form> that
+/// reference it via the form="id" attribute, matching browser behavior.
 pub fn collectForms(
     arena: Allocator,
     root: *Node,
@@ -145,16 +153,14 @@ pub fn collectForms(
 
     var tw = TreeWalker.Full.init(root, .{});
     while (tw.next()) |node| {
-        const el = node.is(Element) orelse continue;
+        const form = node.is(Element.Html.Form) orelse continue;
-        if (el.getTag() != .form) continue;
+        const el = form.asElement();
 
-        const form_el = el.is(Element.Html.Form) orelse continue;
+        const fields = try collectFormFields(arena, form, page);
-
-        const fields = try collectFormFields(arena, node, page);
         if (fields.len == 0) continue;
 
         const action_attr = el.getAttributeSafe(comptime .wrap("action"));
-        const method_str = form_el.getMethod();
+        const method_str = form.getMethod();
 
         try forms.append(arena, .{
             .node = node,
@@ -169,64 +175,71 @@ pub fn collectForms(
 
 fn collectFormFields(
     arena: Allocator,
-    form_node: *Node,
+    form: *Element.Html.Form,
     page: *Page,
 ) ![]FormField {
     var fields: std.ArrayList(FormField) = .empty;
+    const form_node = form.asNode();
 
-    var tw = TreeWalker.Full.init(form_node, .{});
+    var elements = try form.getElements(page);
-    while (tw.next()) |node| {
+    var it = try elements.iterator();
-        const el = node.is(Element) orelse continue;
+    while (it.next()) |el| {
+        const node = el.asNode();
 
-        switch (el.getTag()) {
+        const is_disabled = el.getAttributeSafe(comptime .wrap("disabled")) != null or
-            .input => {
+            isDisabledByFieldset(el, form_node);
-                const input = el.is(Element.Html.Input) orelse continue;
-                if (input._input_type == .hidden) continue;
-                if (input._input_type == .submit or input._input_type == .button or input._input_type == .image) continue;
 
-                try fields.append(arena, .{
+        if (el.is(Element.Html.Input)) |input| {
-                    .node = node,
+            if (input._input_type == .hidden) continue;
-                    .tag_name = "input",
+            if (input._input_type == .submit or input._input_type == .button or input._input_type == .image) continue;
-                    .name = el.getAttributeSafe(comptime .wrap("name")),
-                    .input_type = input._input_type.toString(),
-                    .required = el.getAttributeSafe(comptime .wrap("required")) != null,
-                    .value = input.getValue(),
-                    .placeholder = el.getAttributeSafe(comptime .wrap("placeholder")),
-                    .options = &.{},
-                });
-            },
-            .textarea => {
-                const textarea = el.is(Element.Html.TextArea) orelse continue;
 
-                try fields.append(arena, .{
+            try fields.append(arena, .{
-                    .node = node,
+                .node = node,
-                    .tag_name = "textarea",
+                .tag_name = "input",
-                    .name = el.getAttributeSafe(comptime .wrap("name")),
+                .name = el.getAttributeSafe(comptime .wrap("name")),
-                    .input_type = null,
+                .input_type = input._input_type.toString(),
-                    .required = el.getAttributeSafe(comptime .wrap("required")) != null,
+                .required = el.getAttributeSafe(comptime .wrap("required")) != null,
-                    .value = textarea.getValue(),
+                .disabled = is_disabled,
-                    .placeholder = el.getAttributeSafe(comptime .wrap("placeholder")),
+                .value = input.getValue(),
-                    .options = &.{},
+                .placeholder = el.getAttributeSafe(comptime .wrap("placeholder")),
-                });
+                .options = &.{},
-            },
+            });
-            .select => {
+            continue;
-                const select = el.is(Element.Html.Select) orelse continue;
-
-                const options = try collectSelectOptions(arena, node, page);
-
-                try fields.append(arena, .{
-                    .node = node,
-                    .tag_name = "select",
-                    .name = el.getAttributeSafe(comptime .wrap("name")),
-                    .input_type = null,
-                    .required = el.getAttributeSafe(comptime .wrap("required")) != null,
-                    .value = select.getValue(page),
-                    .placeholder = null,
-                    .options = options,
-                });
-            },
-            else => {},
         }
+
+        if (el.is(Element.Html.TextArea)) |textarea| {
+            try fields.append(arena, .{
+                .node = node,
+                .tag_name = "textarea",
+                .name = el.getAttributeSafe(comptime .wrap("name")),
+                .input_type = null,
+                .required = el.getAttributeSafe(comptime .wrap("required")) != null,
+                .disabled = is_disabled,
+                .value = textarea.getValue(),
+                .placeholder = el.getAttributeSafe(comptime .wrap("placeholder")),
+                .options = &.{},
+            });
+            continue;
+        }
+
+        if (el.is(Element.Html.Select)) |select| {
+            const options = try collectSelectOptions(arena, node, page);
+
+            try fields.append(arena, .{
+                .node = node,
+                .tag_name = "select",
+                .name = el.getAttributeSafe(comptime .wrap("name")),
+                .input_type = null,
+                .required = el.getAttributeSafe(comptime .wrap("required")) != null,
+                .disabled = is_disabled,
+                .value = select.getValue(page),
+                .placeholder = null,
+                .options = options,
+            });
+            continue;
+        }
+
+        // Button elements from getElements() - skip (not fillable)
     }
 
     return fields.items;
@@ -254,6 +267,38 @@ fn collectSelectOptions(
     return options.items;
 }
 
+/// Returns true if `element` is disabled by an ancestor <fieldset disabled>,
+/// stopping at the form boundary.
+/// Per spec, elements inside the first <legend> child of a disabled fieldset
+/// are NOT disabled by that fieldset.
+fn isDisabledByFieldset(element: *Element, form_node: *Node) bool {
+    const element_node = element.asNode();
+    var current: ?*Node = element_node._parent;
+    while (current) |node| {
+        if (node == form_node) {
+            return false;
+        }
+
+        current = node._parent;
+        const el = node.is(Element) orelse continue;
+
+        if (el.getTag() == .fieldset and el.getAttributeSafe(comptime .wrap("disabled")) != null) {
+            var child = el.firstElementChild();
+            while (child) |c| {
+                if (c.getTag() == .legend) {
+                    if (c.asNode().contains(element_node)) {
+                        return false;
+                    }
+                    break;
+                }
+                child = c.nextElementSibling();
+            }
+            return true;
+        }
+    }
+    return false;
+}
+
 const testing = @import("../testing.zig");
 
 fn testForms(html: []const u8) ![]FormInfo {
@@ -283,6 +328,7 @@ test "browser.forms: login form" {
     try testing.expectEqual("email", forms[0].fields[0].name.?);
     try testing.expectEqual("email", forms[0].fields[0].input_type.?);
     try testing.expect(forms[0].fields[0].required);
+    try testing.expect(!forms[0].fields[0].disabled);
     try testing.expectEqual("password", forms[0].fields[1].name.?);
 }
 
@@ -360,3 +406,48 @@ test "browser.forms: multiple forms" {
     try testing.expectEqual(1, forms[0].fields.len);
     try testing.expectEqual(2, forms[1].fields.len);
 }
+
+test "browser.forms: disabled fields flagged" {
+    defer testing.reset();
+    defer testing.test_session.removePage();
+    const forms = try testForms(
+        \\<form>
+        \\  <input type="text" name="enabled_field">
+        \\  <input type="text" name="disabled_field" disabled>
+        \\</form>
+    );
+    try testing.expectEqual(1, forms.len);
+    try testing.expectEqual(2, forms[0].fields.len);
+    try testing.expect(!forms[0].fields[0].disabled);
+    try testing.expect(forms[0].fields[1].disabled);
+}
+
+test "browser.forms: disabled fieldset" {
+    defer testing.reset();
+    defer testing.test_session.removePage();
+    const forms = try testForms(
+        \\<form>
+        \\  <fieldset disabled>
+        \\    <input type="text" name="in_disabled_fieldset">
+        \\  </fieldset>
+        \\  <input type="text" name="outside_fieldset">
+        \\</form>
+    );
+    try testing.expectEqual(1, forms.len);
+    try testing.expectEqual(2, forms[0].fields.len);
+    try testing.expect(forms[0].fields[0].disabled);
+    try testing.expect(!forms[0].fields[1].disabled);
+}
+
+test "browser.forms: external field via form attribute" {
+    defer testing.reset();
+    defer testing.test_session.removePage();
+    const forms = try testForms(
+        \\<input type="text" name="external" form="myform">
+        \\<form id="myform" action="/submit">
+        \\  <input type="text" name="internal">
+        \\</form>
+    );
+    try testing.expectEqual(1, forms.len);
+    try testing.expectEqual(2, forms[0].fields.len);
+}

src/mcp/tools.zig

@@ -455,11 +455,12 @@ fn handleDetectForms(server: *Server, arena: std.mem.Allocator, id: std.json.Val
         url: ?[:0]const u8 = null,
     };
     if (arguments) |args_raw| {
-        if (std.json.parseFromValueLeaky(Params, arena, args_raw, .{ .ignore_unknown_fields = true })) |args| {
+        const args = std.json.parseFromValueLeaky(Params, arena, args_raw, .{ .ignore_unknown_fields = true }) catch {
-            if (args.url) |u| {
+            return server.sendError(id, .InvalidParams, "Invalid arguments for detectForms");
-                try performGoto(server, u, id);
+        };
-            }
+        if (args.url) |u| {
-        } else |_| {}
+            try performGoto(server, u, id);
+        }
     }
     const page = server.session.currentPage() orelse {
         return server.sendError(id, .PageNotLoaded, "Page not loaded");