mirror of
https://github.com/lightpanda-io/browser.git
synced 2026-09-17 17:22:43 -04:00
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.
This commit is contained in:
1 parent
7571d6e7a0
commit
45eb267f5c
2 files changed
+188
-16
No files matched your search
@@ -0,0 +1,170 @@
|
||||
<!DOCTYPE html>
|
||||
<script src="../testing.js"></script>
|
||||
|
||||
<script id=absent_properties_do_not_mutate_style>
|
||||
{
|
||||
for (const initial of [null, '', 'color:red']) {
|
||||
const element = document.createElement('div');
|
||||
if (initial !== null) element.setAttribute('style', initial);
|
||||
const observer = new MutationObserver(() => {});
|
||||
observer.observe(element, { attributes: true });
|
||||
testing.expectEqual('', element.style.removeProperty('display'));
|
||||
element.style.display = '';
|
||||
element.style.setProperty('display', '');
|
||||
element.style.cssFloat = '';
|
||||
element.style.removeProperty('--absent');
|
||||
testing.expectEqual(0, observer.takeRecords().length);
|
||||
testing.expectEqual(initial, element.getAttribute('style'));
|
||||
observer.disconnect();
|
||||
}
|
||||
}
|
||||
</script>
|
||||
|
||||
<script id=identical_declarations_preserve_raw_style>
|
||||
{
|
||||
const element = document.createElement('div');
|
||||
const raw = 'color:red; margin-top:0px; float:left; --token:x';
|
||||
element.setAttribute('style', raw);
|
||||
const observer = new MutationObserver(() => {});
|
||||
observer.observe(element, { attributes: true });
|
||||
element.style.color = 'red';
|
||||
element.style.setProperty('COLOR', 'red');
|
||||
element.style.marginTop = '0';
|
||||
element.style.cssFloat = 'left';
|
||||
element.style.setProperty('--token', 'x');
|
||||
testing.expectEqual(0, observer.takeRecords().length);
|
||||
testing.expectEqual(raw, element.getAttribute('style'));
|
||||
observer.disconnect();
|
||||
}
|
||||
</script>
|
||||
|
||||
<script id=priority_changes_and_empty_values>
|
||||
{
|
||||
const element = document.createElement('div');
|
||||
element.style.setProperty('color', 'red', 'important');
|
||||
const observer = new MutationObserver(() => {});
|
||||
observer.observe(element, { attributes: true, attributeOldValue: true });
|
||||
element.style.setProperty('color', 'red', 'IMPORTANT');
|
||||
testing.expectEqual(0, observer.takeRecords().length);
|
||||
element.style.setProperty('color', 'blue', 'invalid');
|
||||
testing.expectEqual(0, observer.takeRecords().length);
|
||||
testing.expectEqual('red', element.style.color);
|
||||
element.style.setProperty('color', 'red');
|
||||
const priorityRecords = observer.takeRecords();
|
||||
testing.expectEqual(1, priorityRecords.length);
|
||||
testing.expectEqual('color: red !important;', priorityRecords[0].oldValue);
|
||||
testing.expectEqual('', element.style.getPropertyPriority('color'));
|
||||
element.style.setProperty('color', '', 'important');
|
||||
const removed = observer.takeRecords();
|
||||
testing.expectEqual(1, removed.length);
|
||||
testing.expectEqual('color: red;', removed[0].oldValue);
|
||||
testing.expectEqual('', element.getAttribute('style'));
|
||||
element.style.setProperty('color', '', 'important');
|
||||
testing.expectEqual(0, observer.takeRecords().length);
|
||||
observer.disconnect();
|
||||
}
|
||||
</script>
|
||||
|
||||
<script id=real_changes_and_explicit_assignments_still_notify>
|
||||
{
|
||||
const element = document.createElement('div');
|
||||
element.style.color = 'red';
|
||||
const observer = new MutationObserver(() => {});
|
||||
observer.observe(element, { attributes: true, attributeOldValue: true });
|
||||
element.style.color = 'blue';
|
||||
let records = observer.takeRecords();
|
||||
testing.expectEqual(1, records.length);
|
||||
testing.expectEqual('style', records[0].attributeName);
|
||||
testing.expectEqual('color: red;', records[0].oldValue);
|
||||
testing.expectEqual('blue', element.style.color);
|
||||
element.style.cssText = element.style.cssText;
|
||||
testing.expectEqual(1, observer.takeRecords().length);
|
||||
element.setAttribute('style', element.getAttribute('style'));
|
||||
testing.expectEqual(1, observer.takeRecords().length);
|
||||
testing.expectEqual('blue', element.style.removeProperty('COLOR'));
|
||||
records = observer.takeRecords();
|
||||
testing.expectEqual(1, records.length);
|
||||
testing.expectEqual('color: blue;', records[0].oldValue);
|
||||
testing.expectEqual('', element.style.removeProperty('color'));
|
||||
testing.expectEqual(0, observer.takeRecords().length);
|
||||
element.style.cssText = '';
|
||||
testing.expectEqual(1, observer.takeRecords().length);
|
||||
observer.disconnect();
|
||||
}
|
||||
</script>
|
||||
|
||||
<script id=css_float_priority_changes_notify>
|
||||
{
|
||||
const element = document.createElement('div');
|
||||
element.style.setProperty('float', 'left', 'important');
|
||||
const observer = new MutationObserver(() => {});
|
||||
observer.observe(element, { attributes: true });
|
||||
element.style.cssFloat = 'left';
|
||||
testing.expectEqual(1, observer.takeRecords().length);
|
||||
testing.expectEqual('', element.style.getPropertyPriority('float'));
|
||||
element.style.cssFloat = 'left';
|
||||
testing.expectEqual(0, observer.takeRecords().length);
|
||||
observer.disconnect();
|
||||
}
|
||||
</script>
|
||||
|
||||
<script id=custom_property_case_is_significant>
|
||||
{
|
||||
const element = document.createElement('div');
|
||||
element.style.setProperty('--Token', 'x');
|
||||
const observer = new MutationObserver(() => {});
|
||||
observer.observe(element, { attributes: true });
|
||||
element.style.setProperty('--token', 'x');
|
||||
testing.expectEqual(1, observer.takeRecords().length);
|
||||
element.style.setProperty('--Token', 'y');
|
||||
testing.expectEqual(1, observer.takeRecords().length);
|
||||
element.style.setProperty('--Token', 'y');
|
||||
testing.expectEqual(0, observer.takeRecords().length);
|
||||
testing.expectEqual('x', element.style.getPropertyValue('--token'));
|
||||
testing.expectEqual('y', element.style.getPropertyValue('--Token'));
|
||||
observer.disconnect();
|
||||
}
|
||||
</script>
|
||||
|
||||
<script id=observer_reapplying_style_converges>
|
||||
(async () => {
|
||||
const state = await testing.async();
|
||||
const element = document.createElement('div');
|
||||
let calls = 0;
|
||||
const observer = new MutationObserver(() => {
|
||||
if (++calls >= 4) observer.disconnect();
|
||||
element.style.color = 'red';
|
||||
element.style.removeProperty('display');
|
||||
element.style.cssFloat = '';
|
||||
});
|
||||
observer.observe(element, { attributes: true });
|
||||
element.style.color = 'red';
|
||||
await new Promise(resolve => setTimeout(resolve, 0));
|
||||
observer.disconnect();
|
||||
state.resolve();
|
||||
await state.done(() => {
|
||||
testing.expectEqual(1, calls);
|
||||
testing.expectEqual('red', element.style.color);
|
||||
});
|
||||
})();
|
||||
</script>
|
||||
|
||||
<script id=healthy_batches_are_not_disconnected>
|
||||
(async () => {
|
||||
const state = await testing.async();
|
||||
const element = document.createElement('div');
|
||||
let delivered = 0;
|
||||
const observers = Array.from({ length: 64 }, () => {
|
||||
const observer = new MutationObserver(() => delivered++);
|
||||
observer.observe(element, { attributes: true });
|
||||
return observer;
|
||||
});
|
||||
for (let round = 0; round < 32; round++) {
|
||||
element.setAttribute('data-round', String(round));
|
||||
await new Promise(resolve => setTimeout(resolve, 0));
|
||||
}
|
||||
observers.forEach(observer => observer.disconnect());
|
||||
state.resolve();
|
||||
await state.done(() => testing.expectEqual(2048, delivered));
|
||||
})();
|
||||
</script>
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user