Merge pull request #3222 from lightpanda-io/style-attr-resync

webapi: Re-parse element.style when style attribute changes

src/browser/Frame.zig

@@ -2964,6 +2964,8 @@ pub fn attributeChange(self: *Frame, element: *Element, name: String, value: Str
     } else if (name.eql(comptime .wrap("popover"))) {
         const old = if (old_value) |o| o.str() else null;
         popover.attributeChanged(element, old, value.str(), self);
+    } else if (name.eql(comptime .wrap("style"))) {
+        self.styleAttributeChanged(element, value.str());
     }
 }
 
@@ -2985,9 +2987,18 @@ pub fn attributeRemove(self: *Frame, element: *Element, name: String, old_value:
         }
     } else if (name.eql(comptime .wrap("popover"))) {
         popover.attributeChanged(element, old_value.str(), null, self);
+    } else if (name.eql(comptime .wrap("style"))) {
+        self.styleAttributeChanged(element, null);
     }
 }
 
+fn styleAttributeChanged(self: *Frame, element: *Element, value: ?[]const u8) void {
+    const style = self._element_styles.get(element) orelse return;
+    style.asCSSStyleDeclaration().styleAttributeChanged(value, self) catch |err| {
+        log.err(.frame, "style attribute reparse", .{ .err = err, .type = self._type, .url = self.url });
+    };
+}
+
 pub fn signalSlotChange(self: *Frame, slot: *Element.Html.Slot) void {
     self._slots_pending_slotchange.put(self.arena, slot, {}) catch |err| {
         log.err(.frame, "signalSlotChange.put", .{ .err = err, .type = self._type, .url = self.url });

src/browser/StyleManager.zig

@@ -1191,6 +1191,8 @@ const CheckVisibilityOptions = struct {
 // its field max, so a real rule can never pack to all-ones.
 const INLINE_PRIORITY: u64 = std.math.maxInt(u64);
 
+// `frame` is the StyleManager's frame, which callers guarantee is el's owner
+// frame (el.ownerFrame) — the map where the materialized style lives.
 fn getInlineStyleProperty(el: *Element, property_name: String, frame: *Frame) ?*CSSStyleProperty {
     const style = frame._element_styles.get(el) orelse blk: {
         // No JS-set style object and no style attribute -> nothing inline to read.

src/browser/tests/element/css_style_properties.html

@@ -191,3 +191,32 @@
   testing.expectEqual('0.5em 0.25em', div.style.margin);
 }
 </script>
+
+<script id="styleAttributeResync">
+{
+  const div = document.createElement('div');
+  div.setAttribute('style', 'color: red');
+  testing.expectEqual('color: red;', div.style.cssText);
+
+  // attribute writes after .style is materialized must be reflected
+  div.setAttribute('style', 'color: green');
+  testing.expectEqual('color: green;', div.style.cssText);
+  testing.expectEqual('green', div.style.color);
+
+  div.style.backgroundColor = 'blue';
+  testing.expectEqual('color: green; background-color: blue;', div.style.cssText);
+  testing.expectEqual('color: green; background-color: blue;', div.getAttribute('style'));
+
+  div.setAttribute('style', 'margin: 1px');
+  div.removeAttribute('style');
+  testing.expectEqual('', div.style.cssText);
+  testing.expectEqual(0, div.style.length);
+
+  // connected element, same rules
+  const c = $('#test-div');
+  c.setAttribute('style', 'color: red');
+  testing.expectEqual('red', c.style.color);
+  c.setAttribute('style', 'color: teal');
+  testing.expectEqual('teal', c.style.color);
+}
+</script>

src/browser/tests/frames/cross_realm_attributes.html

@@ -80,4 +80,47 @@ reads of iframe data-* returned null (WPT encoding/*).
   });
 }
 </script>
+
+<script id="cross_realm_style_resync">
+  testing.onload(() => {
+    const idf = document.getElementById('idf');
+    const iwin = idf.contentWindow;
+    const el = idf.contentDocument.getElementById('s1');
+
+    // el.style materialized from the parent realm must live in the iframe's
+    // (owner) map: a style attribute write from either realm resyncs it.
+    el.style.color = 'red';
+    testing.expectEqual('color: red;', iwin.readAttr('style'));
+
+    iwin.setAttr('style', 'color: blue');
+    testing.expectEqual('blue', el.style.color);
+
+    el.setAttribute('style', 'color: green');
+    testing.expectEqual('green', el.style.color);
+
+    // and a subsequent parent-side write must not resurrect the stale list
+    el.style.margin = '1px';
+    testing.expectEqual('color: green; margin: 1px;', iwin.readAttr('style'));
+
+    iwin.removeAttr('style');
+    testing.expectEqual('', el.style.cssText);
+    testing.expectEqual(false, iwin.checkHidden());
+
+    // inline display:none set from the parent is seen by the iframe realm's
+    // visibility check (same map, same StyleManager walk)
+    el.style.display = 'none';
+    testing.expectEqual(true, iwin.checkHidden());
+    iwin.removeAttr('style');
+    testing.expectEqual(true, el.checkVisibility());
+
+    // a rule from the iframe's own stylesheet must apply, even when the
+    // check is made from the parent realm (owner frame's StyleManager)
+    el.className = 'iframe-hidden';
+    testing.expectEqual(false, el.checkVisibility());
+    testing.expectEqual('none', iwin.getComputedStyle(el).display);
+    testing.expectEqual('none', getComputedStyle(el).display);
+    el.className = '';
+    testing.expectEqual(true, el.checkVisibility());
+  });
+</script>
 </body>

src/browser/tests/frames/support/cross_realm_attributes.html

@@ -1,4 +1,5 @@
 <!DOCTYPE html>
+<head><style>.iframe-hidden { display: none }</style></head>
 <body>
   <span id="s1" data-cp="5F" data-bytes="A1B2" data-cycle="A1B2" nonstandardattr="ns1">x</span>
   <script>
@@ -12,6 +13,9 @@
     function removeAttr(name) {
       document.getElementById('s1').removeAttribute(name);
     }
+    function checkHidden() {
+      return !document.getElementById('s1').checkVisibility();
+    }
 
     window.addEventListener('message', (e) => {
       if (e.data !== 'cycle-attr') return;

src/browser/webapi/Element.zig

@@ -970,16 +970,20 @@ pub fn getAttributeNamedNodeMap(self: *Element, frame: *Frame) !*Attribute.Named
     return gop.value_ptr.*;
 }
 
+// The materialized style lives in the map of the element's own frame, not
+// the caller's: attributeChange (which resyncs it) is dispatched on the owner
+// frame, and a same-origin script can reach an element in another frame.
 pub fn getOrCreateStyle(self: *Element, frame: *Frame) !*CSSStyleProperties {
-    const gop = try frame._element_styles.getOrPut(frame.arena, self);
+    const owner = self.ownerFrame(frame);
+    const gop = try owner._element_styles.getOrPut(owner.arena, self);
     if (!gop.found_existing) {
-        gop.value_ptr.* = try CSSStyleProperties.init(self, false, frame);
+        gop.value_ptr.* = try CSSStyleProperties.init(self, false, owner);
     }
     return gop.value_ptr.*;
 }
 
 fn getStyle(self: *Element, frame: *Frame) ?*CSSStyleProperties {
-    return frame._element_styles.get(self);
+    return self.ownerFrame(frame)._element_styles.get(self);
 }
 
 pub fn setStyle(self: *Element, value: []const u8, frame: *Frame) !void {
@@ -1299,12 +1303,15 @@ pub const VisibilityCache = StyleManager.VisibilityCache;
 /// Cache for pointer-events checks - re-exported from StyleManager for convenience.
 pub const PointerEventsCache = StyleManager.PointerEventsCache;
 
+// Style checks go through the StyleManager of the element's own frame, not
+// the caller's: its stylesheets and materialized inline styles are per-frame,
+// and a same-origin script can reach an element in another frame.
 pub fn hasPointerEventsNone(self: *Element, cache: ?*PointerEventsCache, frame: *Frame) bool {
-    return frame._style_manager.hasPointerEventsNone(self, cache);
+    return self.ownerFrame(frame)._style_manager.hasPointerEventsNone(self, cache);
 }
 
 pub fn checkVisibilityCached(self: *Element, cache: ?*VisibilityCache, frame: *Frame) bool {
-    return !frame._style_manager.isHidden(self, cache, .{});
+    return !self.ownerFrame(frame)._style_manager.isHidden(self, cache, .{});
 }
 
 const CheckVisibilityOpts = struct {
@@ -1315,7 +1322,7 @@ const CheckVisibilityOpts = struct {
 };
 pub fn checkVisibility(self: *Element, opts_: ?CheckVisibilityOpts, frame: *Frame) bool {
     const opts = opts_ orelse CheckVisibilityOpts{};
-    return !frame._style_manager.isHidden(self, null, .{
+    return !self.ownerFrame(frame)._style_manager.isHidden(self, null, .{
         .check_opacity = opts.checkOpacity or opts.opacityProperty,
         .check_visibility = opts.visibilityProperty or opts.checkVisibilityCSS,
     });

src/browser/webapi/css/CSSStyleDeclaration.zig

@@ -34,6 +34,7 @@ const CSSStyleDeclaration = @This();
 _element: ?*Element = null,
 _properties: std.DoublyLinkedList = .{},
 _is_computed: bool = false,
+_syncing: bool = false,
 
 // Parse the element's existing style attribute into _properties so that
 // subsequent JS reads and writes see all CSS properties, not just newly
@@ -43,11 +44,14 @@ pub fn parseInlineStyle(self: *CSSStyleDeclaration, frame: *Frame) !void {
         return;
     }
     const el = self._element orelse return;
-    if (el.getAttributeSafe(comptime .wrap("style"))) |attr_value| {
-        var it = CssParser.parseDeclarationsList(attr_value);
-        while (it.next()) |declaration| {
-            try self.applyParsedDeclaration(declaration, frame);
-        }
+    const attr_value = el.getAttributeSafe(comptime .wrap("style")) orelse return;
+    try self.applyDeclarations(attr_value, frame);
+}
+
+fn applyDeclarations(self: *CSSStyleDeclaration, text: []const u8, frame: *Frame) !void {
+    var it = CssParser.parseDeclarationsList(text);
+    while (it.next()) |declaration| {
+        try self.applyParsedDeclaration(declaration, frame);
     }
 }
 
@@ -78,10 +82,11 @@ pub fn getPropertyValue(self: *const CSSStyleDeclaration, property_name: []const
     // tree builders (Playwright ariaSnapshot) consult on every element.
     if (self._is_computed) {
         if (self._element) |element| {
+            const style_manager = &element.ownerFrame(frame)._style_manager;
             if (wrapped.eql(comptime .wrap("display"))) {
-                if (frame._style_manager.hasDisplayNone(element)) return "none";
+                if (style_manager.hasDisplayNone(element)) return "none";
             } else if (wrapped.eql(comptime .wrap("visibility"))) {
-                if (frame._style_manager.hasVisibilityHiddenInherited(element)) return "hidden";
+                if (style_manager.hasVisibilityHiddenInherited(element)) return "hidden";
             }
         }
     }
@@ -92,7 +97,7 @@ pub fn getPropertyValue(self: *const CSSStyleDeclaration, property_name: []const
             if (self._element) |element| {
                 // Resolve inline `style=` declarations through the element's
                 // parsed inline style, so computed values match `el.style`.
-                if (frame._style_manager.inlineStyleValue(element, wrapped)) |value| {
+                if (element.ownerFrame(frame)._style_manager.inlineStyleValue(element, wrapped)) |value| {
                     return value;
                 }
 
@@ -213,9 +218,36 @@ fn removePropertyImpl(self: *CSSStyleDeclaration, property_name: []const u8, fra
 fn syncStyleAttribute(self: *CSSStyleDeclaration, frame: *Frame) !void {
     const element = self._element orelse return;
     const css_text = try self.getCssText(frame);
+    self._syncing = true;
+    defer self._syncing = false;
     try element.setAttributeSafe(comptime .wrap("style"), .wrap(css_text), frame);
 }
 
+// The element's style attribute changed (null: removed)
+pub fn styleAttributeChanged(self: *CSSStyleDeclaration, text: ?[]const u8, frame: *Frame) !void {
+    if (self._syncing) {
+        // this was us making the change internally, automatically in-sync
+        return;
+    }
+    // ok, this change was from the outside, e.g. via setAttribute('style', ....)
+    // we need to get this declaration back in sync
+    self.clearProperties(frame);
+    if (text) |t| {
+        try self.applyDeclarations(t, frame);
+    }
+}
+
+fn clearProperties(self: *CSSStyleDeclaration, frame: *Frame) void {
+    var node = self._properties.first;
+    while (node) |n| {
+        const next = n.next;
+        const prop = Property.fromNodeLink(n);
+        self._properties.remove(n);
+        frame._factory.destroy(prop);
+        node = next;
+    }
+}
+
 pub fn getFloat(self: *const CSSStyleDeclaration, frame: *Frame) []const u8 {
     return self.getPropertyValue("float", frame);
 }
@@ -232,21 +264,9 @@ pub fn getCssText(self: *const CSSStyleDeclaration, frame: *Frame) ![]const u8 {
 }
 
 pub fn setCssText(self: *CSSStyleDeclaration, text: []const u8, frame: *Frame) !void {
-    // Clear existing properties
-    var node = self._properties.first;
-    while (node) |n| {
-        const next = n.next;
-        const prop = Property.fromNodeLink(n);
-        self._properties.remove(n);
-        frame._factory.destroy(prop);
-        node = next;
-    }
+    self.clearProperties(frame);
 
-    // Parse and set new properties
-    var it = CssParser.parseDeclarationsList(text);
-    while (it.next()) |declaration| {
-        try self.applyParsedDeclaration(declaration, frame);
-    }
+    try self.applyDeclarations(text, frame);
     try self.syncStyleAttribute(frame);
 }
 

src/browser/webapi/element/svg/TextContent.zig

@@ -88,7 +88,8 @@ fn text(self: *TextContent, frame: *Frame) []const u8 {
 }
 
 fn fontSize(self: *TextContent, frame: *Frame) f64 {
-    return frame._style_manager.computedFontSize(self.asElement());
+    const element = self.asElement();
+    return element.ownerFrame(frame)._style_manager.computedFontSize(element);
 }
 
 fn getTextLength(self: *TextContent, frame: *Frame) !*AnimatedLength {

src/browser/webapi/svg/Length.zig

@@ -290,15 +290,16 @@ fn pageViewportDimension(direction: Direction, frame: *Frame) f64 {
 fn resolveParsedLength(parsed: Parsed, element: *Element, direction: Direction, frame: *Frame, depth: u8) f64 {
     const factor = switch (parsed.unit) {
         .percentage => ancestorViewportDimensionAt(element, direction, frame, depth) / 100.0,
-        .em => frame._style_manager.computedFontSize(element),
-        .ex => frame._style_manager.computedFontSize(element) / 2.0,
+        .em => element.ownerFrame(frame)._style_manager.computedFontSize(element),
+        .ex => element.ownerFrame(frame)._style_manager.computedFontSize(element) / 2.0,
         else => units.absoluteLengthFactor(toShared(parsed.unit)).?,
     };
     return parsed.value * factor;
 }
 
 fn fontSize(self: *const Length, frame: *Frame) f64 {
-    return frame._style_manager.computedFontSize(self._element);
+    const element = self._element orelse return frame._style_manager.computedFontSize(null);
+    return element.ownerFrame(frame)._style_manager.computedFontSize(element);
 }
 
 const Parsed = struct {