From dfa0ba778d891faa147d0404bf7e6f571cc0dee7 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 29 Apr 2026 00:53:10 +0200 Subject: [PATCH] css: address review feedback on UA display:none helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Tighten matchesUaDisplayNoneRule docstring; lead with the centralization purpose and HTML Rendering §15.3.1 spec citation. - Reorder checks: tag-name UA list first (O(1) switch, exits for ~95% of elements with ordinary tags before the attribute lookup). - Use el.is(Input) + input._input_type == .hidden instead of case-insensitive string compare on the raw "type" attribute; matches the pattern used in EventManager / Form / RadioNodeList, and _input_type is parsed case-insensitively at attribute-set time. - Compress the comment block above the matchesUaDisplayNoneRule call site in isElementHidden. Behavior-preserving refactor; full unit test suite (494 tests) passes. --- src/browser/StyleManager.zig | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/browser/StyleManager.zig b/src/browser/StyleManager.zig index 35e436cc..723a9a51 100644 --- a/src/browser/StyleManager.zig +++ b/src/browser/StyleManager.zig @@ -45,6 +45,7 @@ pub const PointerEventsCache = std.AutoHashMapUnmanaged(*Element, bool); const StyleManager = @This(); const Tag = Element.Tag; +const Input = Element.Html.Input; const RuleList = std.MultiArrayList(VisibilityRule); frame: *Frame, @@ -241,23 +242,22 @@ pub fn hasDisplayNone(self: *StyleManager, el: *Element) bool { return self.isElementHidden(el, .{}); } -/// True if `el` matches a UA-stylesheet display:none rule per HTML Rendering -/// §15.3.1 "Hidden elements" — covers the `[hidden]` attribute, the unrendered -/// tag-name list, ``, and the closed-`
` non- -/// `` direct-child rule. Centralized so `getComputedStyle().display` -/// (via `hasDisplayNone`) and `el.checkVisibility()` (via `isHidden`) share -/// the same UA-stylesheet truth. +/// Centralizes UA-stylesheet display:none truth so `getComputedStyle().display` +/// (via `hasDisplayNone`) and `el.checkVisibility()` (via `isHidden`) agree. +/// Spec: HTML Rendering §15.3.1 "Hidden elements". fn matchesUaDisplayNoneRule(el: *Element) bool { - // [hidden] { display: none } — applies to all elements. - if (el.hasAttributeSafe(comptime .wrap("hidden"))) return true; - + // Tag check first: O(1) switch, exits for the ~95% of elements with + // ordinary tags before we touch the attribute list. const tag = el.getTag(); if (tag.isHiddenByUaStylesheet()) return true; + if (el.hasAttributeSafe(comptime .wrap("hidden"))) return true; + // input[type="hidden" i] { display: none !important } + // _input_type is parsed case-insensitively at attribute-set time. if (tag == .input) { - if (el.getAttributeSafe(comptime .wrap("type"))) |type_val| { - if (std.ascii.eqlIgnoreCase(type_val, "hidden")) return true; + if (el.is(Input)) |input| { + if (input._input_type == .hidden) return true; } } @@ -345,14 +345,11 @@ fn isElementHidden(self: *StyleManager, el: *Element, options: CheckVisibilityOp } // UA stylesheet display:none rules (HTML Rendering §15.3.1 "Hidden elements"). - // Inline `display` already wins above (sets display_priority == INLINE_PRIORITY); - // skip the UA check in that case so author inline overrides keep working. - // Otherwise short-circuit return true: the spec lists `input[type=hidden]` - // and `noscript` as `!important` and the rest are normal-origin UA rules - // that author CSS could in principle override, but author overrides of - // `