From 0ffca7fa0a7cff89ade54c5d59b9fd172cfbaea4 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Tue, 22 Sep 2026 11:25:52 +0800
Subject: [PATCH] webapi: element.focus() only on focusable elements

Element.focus() would "focus" the element even when it shouldn't. We already
have the logic to determine if an element is focusable in `user_input.zig`, so
this was moved to Element and is now used in el.focus().
---
 src/browser/frame/user_input.zig      |  46 ++---------
 src/browser/tests/document/focus.html | 109 ++++++++++++++++++++++++++
 src/browser/webapi/Element.zig        |  52 +++++++++---
 3 files changed, 154 insertions(+), 53 deletions(-)

diff --git a/src/browser/frame/user_input.zig b/src/browser/frame/user_input.zig
index 961a47ff0..92c29d59e 100644
--- a/src/browser/frame/user_input.zig
+++ b/src/browser/frame/user_input.zig
@@ -509,7 +509,7 @@ fn deltaToScroll(d: f64) i32 {
 fn hasClickActivationBehavior(node: *Node) bool {
     const element = node.is(Element) orelse return false;
 
-    const html_element = element.is(Element.Html) orelse return isSvgLink(element);
+    const html_element = element.is(Element.Html) orelse return element.isSvgLink();
 
     return switch (html_element._type) {
         .anchor => element.getAttributeInterned("href") != null,
@@ -519,36 +519,11 @@ fn hasClickActivationBehavior(node: *Node) bool {
     };
 }
 
-// SVG 2 <a> links via `href`; xlink:href is the deprecated SVG 1.1 spelling.
-fn svgAnchorHref(element: *Element) ?[]const u8 {
-    return element.getAttributeInterned("href") orelse element.getAttributeSafe(comptime .wrap("xlink:href"));
-}
-
-fn isSvgLink(element: *Element) bool {
-    return element.is(Element.Svg.Graphics.A) != null and svgAnchorHref(element) != null;
-}
-
-/// Focusable without a tabindex attribute.
-fn isNativelyFocusable(el: *Element) bool {
-    if (el.is(Element.Html) == null) {
-        return isSvgLink(el);
-    }
-    return switch (el.getTag()) {
-        .button, .select, .textarea, .iframe => true,
-        .input => el.as(Element.Html.Input)._input_type != .hidden,
-        .anchor, .area => el.getAttributeInterned("href") != null,
-        else => false,
-    };
-}
-
 // Clicks on editable content are for editing: they don't activate the
 // element or any enclosing link.
-// "contenteditable" is 15 bytes — past the comptime SSO limit — so the
-// String wrap runs at runtime, mirroring Html.getIsContentEditable.
 fn isEditingHost(node: *Node) bool {
     const element = node.is(Element) orelse return false;
-    const value = element.getAttributeSafe(.wrap("contenteditable")) orelse return false;
-    return std.ascii.eqlIgnoreCase(value, "false") == false;
+    return element.isEditingHost();
 }
 
 fn outermostEditingHost(target: *Element) ?*Element {
@@ -570,17 +545,6 @@ fn outermostEditingHost(target: *Element) ?*Element {
     return host.is(Element);
 }
 
-/// Unlike sequential focus, a negative tabindex is still mouse-focusable, and
-/// an unparsable one counts as absent (HTML §6.6.3), not as "not focusable".
-fn isMouseFocusable(el: *Element) bool {
-    if (el.isDisabled()) return false;
-
-    if (el.getAttributeInterned("tabindex")) |attr| {
-        if (Element.Html.parseInteger(attr) != null) return true;
-    }
-    return isNativelyFocusable(el);
-}
-
 /// Mousedown default action. A mousedown outside any focusable element moves
 /// focus to the body.
 pub fn focusForMouseDown(frame: *Frame, target: *Element) !void {
@@ -592,7 +556,9 @@ pub fn focusForMouseDown(frame: *Frame, target: *Element) !void {
     var node: ?*Node = target.asNode();
     while (node) |n| : (node = n._parent) {
         const el = n.is(Element) orelse continue;
-        if (isMouseFocusable(el)) {
+        // Unlike sequential focus navigation, a negative tabindex is still
+        // mouse-focusable, so any focusable area qualifies.
+        if (el.focusTabIndex() != null) {
             try el.focus(frame);
             return;
         }
@@ -690,7 +656,7 @@ pub fn handleClick(frame: *Frame, target: *Node, event_target: *Node) !void {
     const element = target.is(Element) orelse return;
 
     if (element.is(Element.Svg.Graphics.A) != null) {
-        const href = svgAnchorHref(element) orelse return;
+        const href = element.svgAnchorHref() orelse return;
         const target_name = element.getAttributeInterned("target") orelse "";
         return followLink(frame, target, element, href, target_name);
     }
diff --git a/src/browser/tests/document/focus.html b/src/browser/tests/document/focus.html
index 6e09469b3..474b5de82 100644
--- a/src/browser/tests/document/focus.html
+++ b/src/browser/tests/document/focus.html
@@ -354,3 +354,112 @@
   wrapper.remove();
 }
 </script>
+
+<script id="focus_non_focusable_noop">
+{
+  // Only a focusable area can take focus: focus() on anything else leaves
+  // document.activeElement (and the element that holds it) alone.
+  const input1 = $('#input1');
+  input1.focus();
+
+  let blurCount = 0;
+  input1.addEventListener('blur', () => blurCount++);
+
+  const div = document.createElement('div');
+  document.body.appendChild(div);
+  div.focus();
+  testing.expectEqual(input1, document.activeElement);
+
+  // An unparsable tabindex counts as absent, so the div stays unfocusable
+  div.setAttribute('tabindex', 'nope');
+  div.focus();
+  testing.expectEqual(input1, document.activeElement);
+
+  const disabled = document.createElement('input');
+  disabled.disabled = true;
+  document.body.appendChild(disabled);
+  disabled.focus();
+  testing.expectEqual(input1, document.activeElement);
+
+  // An <a> without href isn't focusable either
+  const anchor = document.createElement('a');
+  anchor.textContent = 'no href';
+  document.body.appendChild(anchor);
+  anchor.focus();
+  testing.expectEqual(input1, document.activeElement);
+
+  testing.expectEqual(0, blurCount);
+
+  div.remove();
+  disabled.remove();
+  anchor.remove();
+}
+</script>
+
+<script id="focus_tabindex_makes_focusable">
+{
+  const div = document.createElement('div');
+  // A negative tabindex is skipped by Tab, but focus() still works
+  div.setAttribute('tabindex', '-1');
+  document.body.appendChild(div);
+
+  let focusCount = 0;
+  div.addEventListener('focus', () => focusCount++);
+
+  div.focus();
+  testing.expectEqual(div, document.activeElement);
+  testing.expectEqual(1, focusCount);
+
+  div.remove();
+}
+</script>
+
+<script id="focus_contenteditable">
+{
+  const div = document.createElement('div');
+  div.setAttribute('contenteditable', '');
+  const span = document.createElement('span');
+  span.textContent = 'child';
+  div.appendChild(span);
+  document.body.appendChild(div);
+
+  div.focus();
+  testing.expectEqual(div, document.activeElement);
+
+  // A descendant of an editing host is editable, but not itself a focusable
+  // area: the attribute has to be on the element
+  span.focus();
+  testing.expectEqual(div, document.activeElement);
+
+  // contenteditable="false" is not an editing host
+  const plain = document.createElement('div');
+  plain.setAttribute('contenteditable', 'false');
+  document.body.appendChild(plain);
+  plain.focus();
+  testing.expectEqual(div, document.activeElement);
+
+  div.remove();
+  plain.remove();
+}
+</script>
+
+<script id="focus_svg_link">
+{
+  const input1 = $('#input1');
+  input1.focus();
+
+  const wrapper = document.createElement('div');
+  wrapper.innerHTML = '<svg><a id="svg-link" href="#x"><rect/></a><a id="svg-plain"><rect/></a></svg>';
+  document.body.appendChild(wrapper);
+
+  // An SVG <a> without href is no more focusable than an HTML one
+  $('#svg-plain').focus();
+  testing.expectEqual(input1, document.activeElement);
+
+  const link = $('#svg-link');
+  link.focus();
+  testing.expectEqual(link, document.activeElement);
+
+  wrapper.remove();
+}
+</script>
diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig
index f3a644b45..f21ac7ffd 100644
--- a/src/browser/webapi/Element.zig
+++ b/src/browser/webapi/Element.zig
@@ -1181,6 +1181,35 @@ pub fn remove(self: *Element, frame: *Frame) void {
     frame.removeNode(parent, node, .{ .reconnect_to = null });
 }
 
+// SVG 2 <a> links via `href`; xlink:href is the deprecated SVG 1.1 spelling.
+pub fn svgAnchorHref(self: *Element) ?[]const u8 {
+    return self.getAttributeInterned("href") orelse self.getAttributeSafe(comptime .wrap("xlink:href"));
+}
+
+pub fn isSvgLink(self: *Element) bool {
+    return self.is(Svg.Graphics.A) != null and self.svgAnchorHref() != null;
+}
+
+// An editing host takes focus like a form control does.
+pub fn isEditingHost(self: *Element) bool {
+    const value = self.getAttributeSafe(.wrap("contenteditable")) orelse return false;
+    return std.ascii.eqlIgnoreCase(value, "false") == false;
+}
+
+/// Focusable without a tabindex attribute.
+fn isNativelyFocusable(self: *Element) bool {
+    if (self.is(Html) == null) {
+        return self.isSvgLink();
+    }
+
+    return switch (self.getTag()) {
+        .button, .select, .textarea, .iframe => true,
+        .input => self.as(Html.Input)._input_type != .hidden,
+        .anchor, .area => self.getAttributeInterned("href") != null,
+        else => false,
+    };
+}
+
 // The tabindex of a focusable area, or null when the element can't take focus
 // at all. A negative value is still focusable, just skipped by sequential
 // focus navigation.
@@ -1189,20 +1218,19 @@ pub fn focusTabIndex(self: *Element) ?i32 {
     if (self.isDisabled()) {
         return null;
     }
-    if (self.is(Html) == null) {
-        return null;
-    }
 
     if (self.getAttributeInterned("tabindex")) |attr| {
-        return Html.parseInteger(attr) orelse 0;
+        if (Html.parseInteger(attr)) |tab_index| {
+            return tab_index;
+        } else {
+            // can't be parsed is treated the same as no tabindex
+        }
     }
 
-    return switch (self.getTag()) {
-        .button, .select, .textarea, .iframe => 0,
-        .input => if (self.as(Html.Input)._input_type != .hidden) 0 else null,
-        .anchor, .area => if (self.getAttributeInterned("href") != null) 0 else null,
-        else => null,
-    };
+    if (self.isNativelyFocusable() or self.isEditingHost()) {
+        return 0;
+    }
+    return null;
 }
 
 // A focusable area that can take focus right now: connected and being rendered.
@@ -1229,9 +1257,7 @@ pub fn focus(self: *Element, frame: *Frame) !void {
         return;
     }
 
-    // Per HTML spec §6.4.4, an element must be "being rendered" (not
-    // display:none on self or any ancestor) to be focusable.
-    if (!self.isVisible(owner)) {
+    if (self.isFocusable(owner) == false) {
         return;
     }