From 45eb267f5caee18980a1aab44fbedc0291c992a7 Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Mon, 14 Sep 2026 23:11:46 -0400 Subject: [PATCH] cssom: only sync the style attribute when a declaration changes setProperty, removeProperty and cssFloat rewrote the style attribute unconditionally, so assigning a property its current value, or removing one that was never set, produced an attribute mutation record. Chromium emits none in those cases. A storefront extension reacts to attribute mutations by rerendering and reapplying styles. These spurious records keep that cycle running until the watchdog terminates the page. Compare the normalized value and priority before rewriting, and treat removing an absent property as a no-op. Explicit cssText and setAttribute assignments still notify. Test mutation counts, priority-only changes, raw attribute preservation, observer convergence, and healthy batches exceeding 1600 callbacks. --- .../tests/mutation_observer/css_noop.html | 170 ++++++++++++++++++ .../webapi/css/CSSStyleDeclaration.zig | 34 ++-- 2 files changed, 188 insertions(+), 16 deletions(-) create mode 100644 src/browser/tests/mutation_observer/css_noop.html diff --git a/src/browser/tests/mutation_observer/css_noop.html b/src/browser/tests/mutation_observer/css_noop.html new file mode 100644 index 000000000..95b687d7b --- /dev/null +++ b/src/browser/tests/mutation_observer/css_noop.html @@ -0,0 +1,170 @@ + + + + + + + + + + + + + + + + + + diff --git a/src/browser/webapi/css/CSSStyleDeclaration.zig b/src/browser/webapi/css/CSSStyleDeclaration.zig index 65f04b4d3..012c50ae8 100644 --- a/src/browser/webapi/css/CSSStyleDeclaration.zig +++ b/src/browser/webapi/css/CSSStyleDeclaration.zig @@ -166,9 +166,9 @@ pub fn setProperty(self: *CSSStyleDeclaration, property_name: []const u8, value: break :blk true; } else false; - try self.setPropertyImpl(property_name, value, important, frame); - - try self.syncStyleAttribute(frame); + if (try self.setPropertyImpl(property_name, value, important, frame)) { + try self.syncStyleAttribute(frame); + } } /// Apply one declaration parsed from a `style=` block. Unlike the imperative @@ -181,7 +181,7 @@ fn applyParsedDeclaration(self: *CSSStyleDeclaration, declaration: CssParser.Dec if (existing._important) return; } } - try self.setPropertyImpl(declaration.name, declaration.value, declaration.important, frame); + _ = try self.setPropertyImpl(declaration.name, declaration.value, declaration.important, frame); } fn initOwnedString(allocator: Allocator, value: []const u8) !String { @@ -190,10 +190,9 @@ fn initOwnedString(allocator: Allocator, value: []const u8) !String { return String.wrap(try allocator.dupe(u8, value)); } -fn setPropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, value: []const u8, important: bool, frame: *Frame) !void { +fn setPropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, value: []const u8, important: bool, frame: *Frame) !bool { if (value.len == 0) { - _ = try self.removePropertyImpl(property_name, frame); - return; + return (try self.removePropertyImpl(property_name, frame)) != null; } const normalized = normalizePropertyName(property_name, &frame.buf); @@ -203,12 +202,13 @@ fn setPropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, value: // Find existing property if (self.findProperty(.wrap(normalized))) |existing| { + if (existing._value.eql(.wrap(normalized_value)) and existing._important == important) return false; const allocator = frame._factory.storageAllocator(); const new_value = try initOwnedString(allocator, normalized_value); existing._value.deinit(allocator); existing._value = new_value; existing._important = important; - return; + return true; } // Create new property @@ -219,20 +219,21 @@ fn setPropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, value: ._important = important, }); self._properties.append(&prop._node); + return true; } pub fn removeProperty(self: *CSSStyleDeclaration, property_name: []const u8, frame: *Frame) ![]const u8 { if (self._is_computed) { return error.NoModificationAllowed; } - const result = try self.removePropertyImpl(property_name, frame); + const result = (try self.removePropertyImpl(property_name, frame)) orelse return ""; try self.syncStyleAttribute(frame); return result; } -fn removePropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, frame: *Frame) ![]const u8 { +fn removePropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, frame: *Frame) !?[]const u8 { const normalized = normalizePropertyName(property_name, &frame.buf); - const prop = self.findProperty(.wrap(normalized)) orelse return ""; + const prop = self.findProperty(.wrap(normalized)) orelse return null; // the value might not be on the heap (it could be inlined in the small string // optimization), so we need to dupe it. @@ -287,8 +288,9 @@ fn setFloat(self: *CSSStyleDeclaration, value_: ?[]const u8, frame: *Frame) !voi if (self._is_computed) { return error.NoModificationAllowed; } - try self.setPropertyImpl("float", value_ orelse "", false, frame); - try self.syncStyleAttribute(frame); + if (try self.setPropertyImpl("float", value_ orelse "", false, frame)) { + try self.syncStyleAttribute(frame); + } } fn getCssText(self: *const CSSStyleDeclaration, frame: *Frame) ![]const u8 { @@ -990,10 +992,10 @@ test "CSS property value storage is reused" { var style = CSSStyleDeclaration{}; defer style.clearProperties(frame); - try style.setPropertyImpl("transform", "translate3d(1px,0,0)", false, frame); + try testing.expect(try style.setPropertyImpl("transform", "translate3d(1px,0,0)", false, frame)); const first_ptr = style.findProperty(comptime .wrap("transform")).?._value.suffix.ptr; - try style.setPropertyImpl("transform", "translate3d(2px,0,0)", false, frame); - try style.setPropertyImpl("transform", "translate3d(3px,0,0)", false, frame); + try testing.expect(try style.setPropertyImpl("transform", "translate3d(2px,0,0)", false, frame)); + try testing.expect(try style.setPropertyImpl("transform", "translate3d(3px,0,0)", false, frame)); const property = style.findProperty(comptime .wrap("transform")).?; try testing.expectEqual(first_ptr, property._value.suffix.ptr);