From 577a405f61ee48add7cf6904dbd2a26278c6aeff Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Tue, 28 Apr 2026 04:19:06 +0200 Subject: [PATCH] forms: address review feedback on range step matching - Drop redundant `step_attr.len == 3` guard in `snapToStep`. Zig's `std.ascii.eqlIgnoreCase` already short-circuits on length mismatch (`ascii.zig:329`), so the outer length check is wasted work. - Implement the `value` content attribute fallback for step base. Per https://html.spec.whatwg.org/multipage/input.html#concept-input-min the chain is `min` content attr -> `value` content attr -> 0. The fallback was previously skipped (step base went straight from `min` to 0), which meant `` then `el.value = '5.3'` snapped to `5` instead of the spec-required `5.5`. Threads `value_attr` through `sanitizeRange` so the fallback engages whenever `min` is absent or unparseable but a parseable `value` content attr exists. Three new test cases cover the fallback path (s10), the unparseable `value` attr that falls through to 0 (s11), and `min` taking precedence over `value` attr (s12). --- .../tests/element/html/input-attrs.html | 30 +++++++++++++++++++ src/browser/webapi/element/html/Input.zig | 22 ++++++++++---- 2 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/browser/tests/element/html/input-attrs.html b/src/browser/tests/element/html/input-attrs.html index 63ec17a7..89372766 100644 --- a/src/browser/tests/element/html/input-attrs.html +++ b/src/browser/tests/element/html/input-attrs.html @@ -255,5 +255,35 @@ s9.step = '7'; // ladder: 0, 7, 14, ..., 98; midpoint=50; nearest is 49 s9.value = 'garbage'; testing.expectEqual('49', s9.value); + + // Step base falls back through `min` content attr -> `value` content attr -> 0 + // (https://html.spec.whatwg.org/multipage/input.html#concept-input-min). + // When `min` is absent but a parseable `value` content attribute exists, + // it becomes the step base — so the ladder shifts. + const s10 = document.createElement('input'); + s10.type = 'range'; + s10.max = '10'; + s10.setAttribute('value', '3.5'); // content attr only; step_base = 3.5 + s10.value = '5.3'; // ladder [3.5, 4.5, 5.5, ...]; nearest is 5.5 + testing.expectEqual('5.5', s10.value); + + // Unparseable `value` content attr fails the fallback chain and step base + // defaults to 0 (ladder [0, 1, 2, ...]; nearest to 5.3 is 5). + const s11 = document.createElement('input'); + s11.type = 'range'; + s11.max = '10'; + s11.setAttribute('value', 'garbage'); + s11.value = '5.3'; + testing.expectEqual('5', s11.value); + + // `min` content attr wins over `value` content attr in the fallback chain + // (step_base = 0 from min, not 3.5 from value attr). + const s12 = document.createElement('input'); + s12.type = 'range'; + s12.min = '0'; + s12.max = '10'; + s12.setAttribute('value', '3.5'); + s12.value = '5.3'; + testing.expectEqual('5', s12.value); } diff --git a/src/browser/webapi/element/html/Input.zig b/src/browser/webapi/element/html/Input.zig index e8ac31a0..f5fe00d5 100644 --- a/src/browser/webapi/element/html/Input.zig +++ b/src/browser/webapi/element/html/Input.zig @@ -564,7 +564,10 @@ fn sanitizeValue(self: *Input, comptime dupe: bool, value: []const u8, frame: *F .time => return if (isValidTime(value)) if (comptime dupe) try frame.dupeString(value) else value else "", .@"datetime-local" => return try sanitizeDatetimeLocal(dupe, value, frame.arena), .number => return if (isValidFloatingPoint(value)) if (comptime dupe) try frame.dupeString(value) else value else "", - .range => return try sanitizeRange(dupe, value, self.getMin(), self.getMax(), self.getStep(), frame), + .range => { + const value_attr = self.asConstElement().getAttributeSafe(comptime .wrap("value")) orelse ""; + return try sanitizeRange(dupe, value, self.getMin(), self.getMax(), self.getStep(), value_attr, frame); + }, .color => { if (value.len == 7 and value[0] == '#') { var needs_lower = false; @@ -798,13 +801,16 @@ fn sanitizeDatetimeLocal(comptime dupe: bool, value: []const u8, arena: std.mem. /// neighbor instead. /// `min`/`max` default to 0 and 100 respectively when the attribute is missing /// or fails to parse as a valid floating-point number. `step` defaults to 1; -/// `step="any"` (case-insensitive) disables step matching. +/// `step="any"` (case-insensitive) disables step matching. The step base +/// (https://html.spec.whatwg.org/multipage/input.html#concept-input-min) falls +/// back through `min` content attr → `value` content attr → 0. fn sanitizeRange( comptime dupe: bool, value: []const u8, min_attr: []const u8, max_attr: []const u8, step_attr: []const u8, + value_attr: []const u8, frame: *Frame, ) ![]const u8 { const min: f64 = if (isValidFloatingPoint(min_attr)) @@ -815,16 +821,22 @@ fn sanitizeRange( std.fmt.parseFloat(f64, max_attr) catch 100 else 100; + const step_base: f64 = if (isValidFloatingPoint(min_attr)) + std.fmt.parseFloat(f64, min_attr) catch 0 + else if (isValidFloatingPoint(value_attr)) + std.fmt.parseFloat(f64, value_attr) catch 0 + else + 0; if (!isValidFloatingPoint(value)) { - return try formatFloat(frame.arena, snapToStep(min + (max - min) / 2, min, max, min, step_attr)); + return try formatFloat(frame.arena, snapToStep(min + (max - min) / 2, min, max, step_base, step_attr)); } const v0 = std.fmt.parseFloat(f64, value) catch unreachable; // grammar already validated var v = v0; if (v < min) v = min; if (v > max) v = max; - const snapped = snapToStep(v, min, max, min, step_attr); + const snapped = snapToStep(v, min, max, step_base, step_attr); if (v == v0 and snapped == v) { // Already valid and on the ladder — preserve the original string so // assignments like `el.value = "1.0"` round-trip without canonicalizing. @@ -839,7 +851,7 @@ fn sanitizeRange( /// unchanged for `step="any"` (case-insensitive) or when no ladder rung lands /// in `[min, max]`. fn snapToStep(value: f64, min: f64, max: f64, step_base: f64, step_attr: []const u8) f64 { - if (step_attr.len == 3 and std.ascii.eqlIgnoreCase(step_attr, "any")) return value; + if (std.ascii.eqlIgnoreCase(step_attr, "any")) return value; const step: f64 = blk: { if (isValidFloatingPoint(step_attr)) {