CSS Typed OM doesn't schedule mutation records

https://bugs.webkit.org/show_bug.cgi?id=267063 Reviewed by Antti Koivisto. This PR fixes the bug that mutating element's inline style with attributeStyleMap does not enqueue mutation record for mutation observers. To do this, this PR extracts StyleAttributeMutationScope out of PropertySetCSSStyleDeclaration.cpp into its own cpp/h files and generalizes it to support both CSSStyleDeclaration and attributeStyleMap. * LayoutTests/fast/custom-elements/mutation-observer-for-style-attribute-on-custom-elements-expected.txt: Added. * LayoutTests/fast/custom-elements/mutation-observer-for-style-attribute-on-custom-elements.html: Added. * LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt: * LayoutTests/fast/dom/MutationObserver/observe-attributes.html: Added test cases. * Source/WebCore/Sources.txt: * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp: (WebCore::PropertySetCSSStyleDeclaration::setCssText): (WebCore::PropertySetCSSStyleDeclaration::setProperty): (WebCore::PropertySetCSSStyleDeclaration::removeProperty): (WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal): (WebCore::InlineCSSStyleDeclaration::didMutate): (WebCore::StyleAttributeMutationScope::StyleAttributeMutationScope): Deleted. (WebCore::StyleAttributeMutationScope::~StyleAttributeMutationScope): Deleted. (WebCore::StyleAttributeMutationScope::enqueueMutationRecord): Deleted. (WebCore::StyleAttributeMutationScope::didInvalidateStyleAttr): Deleted. * Source/WebCore/css/StyleAttributeMutationScope.cpp: Added. * Source/WebCore/css/StyleAttributeMutationScope.h: Added. (WebCore::StyleAttributeMutationScope::StyleAttributeMutationScope): Added. (WebCore::StyleAttributeMutationScope::~StyleAttributeMutationScope): Added. (WebCore::StyleAttributeMutationScope::enqueueMutationRecord): Added. * Source/WebCore/css/typedom/InlineStylePropertyMap.cpp: (WebCore::InlineStylePropertyMap::removeProperty): (WebCore::InlineStylePropertyMap::setShorthandProperty): (WebCore::InlineStylePropertyMap::setProperty): (WebCore::InlineStylePropertyMap::setCustomProperty): (WebCore::InlineStylePropertyMap::removeCustomProperty): (WebCore::InlineStylePropertyMap::clear): * Source/WebCore/css/typedom/InlineStylePropertyMap.h: Canonical link: https://commits.webkit.org/276782@main
WebKit · Mar 28, 2024 · 73bff01 · 73bff01
1 parent 4c27f79
commit 73bff01
Show file tree

Hide file tree

Showing 10 changed files with 428 additions and 111 deletions.
diff --git a/...ast/custom-elements/mutation-observer-for-style-attribute-on-custom-elements-expected.txt b/...ast/custom-elements/mutation-observer-for-style-attribute-on-custom-elements-expected.txt
@@ -0,0 +1,30 @@
+This tests observing a custom element, which observes "style" content attribute, with a MutationObserver.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS observer.takeRecords().length is 0
+PASS attributeChanges.length is 0
+element = document.createElement("some-element"); observer.observe(element, {attributes: true});
+element.style.width = "100px";
+PASS records = observer.takeRecords(); records.length is 1
+PASS records[0].target is element
+PASS records[0].type is "attributes"
+PASS records[0].oldValue is null
+PASS attributeChanges.length is 1
+PASS attributeChanges[0].name is "style"
+PASS attributeChanges[0].oldValue is null
+PASS attributeChanges[0].newValue is "width: 100px;"
+element.style.color = "red";
+PASS records = observer.takeRecords(); records.length is 1
+PASS records[0].target is element
+PASS records[0].type is "attributes"
+PASS records[0].oldValue is null
+PASS attributeChanges.length is 2
+PASS attributeChanges[1].name is "style"
+PASS attributeChanges[1].oldValue is "width: 100px;"
+PASS attributeChanges[1].newValue is "width: 100px; color: red;"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/...tTests/fast/custom-elements/mutation-observer-for-style-attribute-on-custom-elements.html b/...tTests/fast/custom-elements/mutation-observer-for-style-attribute-on-custom-elements.html
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description('This tests observing a custom element, which observes "style" content attribute, with a MutationObserver.');
+
+let attributeChanges = [];
+class SomeElement extends HTMLElement {
+    static observedAttributes = ["style"]; 
+    constructor() {
+        super();
+    }
+    attributeChangedCallback(name, oldValue, newValue) {
+        attributeChanges.push({name, oldValue, newValue});
+    }
+};
+customElements.define('some-element', SomeElement);
+
+const observer = new MutationObserver((records) => { });
+shouldBe('observer.takeRecords().length', '0');
+shouldBe('attributeChanges.length', '0');
+evalAndLog('element = document.createElement("some-element"); observer.observe(element, {attributes: true});');
+evalAndLog('element.style.width = "100px";');
+shouldBe('records = observer.takeRecords(); records.length', '1');
+shouldBe('records[0].target', 'element');
+shouldBeEqualToString('records[0].type', 'attributes');
+shouldBe('records[0].oldValue', 'null');
+shouldBe('attributeChanges.length', '1');
+shouldBeEqualToString('attributeChanges[0].name', 'style');
+shouldBe('attributeChanges[0].oldValue', 'null');
+shouldBeEqualToString('attributeChanges[0].newValue', 'width: 100px;');
+evalAndLog('element.style.color = "red";');
+shouldBe('records = observer.takeRecords(); records.length', '1');
+shouldBe('records[0].target', 'element');
+shouldBeEqualToString('records[0].type', 'attributes');
+shouldBe('records[0].oldValue', 'null');
+shouldBe('attributeChanges.length', '2');
+shouldBeEqualToString('attributeChanges[1].name', 'style');
+shouldBeEqualToString('attributeChanges[1].oldValue', 'width: 100px;');
+shouldBeEqualToString('attributeChanges[1].newValue', 'width: 100px; color: red;');
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt b/LayoutTests/fast/dom/MutationObserver/observe-attributes-expected.txt
@@ -183,6 +183,37 @@ PASS mutations is null
 Testing that a no-op style property mutation does not create Mutation Records.
 PASS mutations is null
 
+Testing that modifying an elements attiuteStyleMap property dispatches Mutation Records.
+PASS mutations.length is 3
+PASS mutations[0].type is "attributes"
+PASS mutations[0].attributeName is "style"
+PASS mutations[0].oldValue is null
+PASS mutations[1].type is "attributes"
+PASS mutations[1].attributeName is "style"
+PASS mutations[1].oldValue is null
+PASS mutations[2].type is "attributes"
+PASS mutations[2].attributeName is "style"
+PASS mutations[2].oldValue is null
+...mutation record created.
+PASS mutations is null
+
+Testing that modifying an elements attiuteStyleMap property dispatches Mutation Records with correct oldValues.
+PASS mutations.length is 3
+PASS mutations[0].type is "attributes"
+PASS mutations[0].attributeName is "style"
+PASS mutations[0].oldValue is "color: yellow; height: 200px;"
+PASS mutations[1].type is "attributes"
+PASS mutations[1].attributeName is "style"
+PASS mutations[1].oldValue is "color: red; height: 200px;"
+PASS mutations[2].type is "attributes"
+PASS mutations[2].attributeName is "style"
+PASS mutations[2].oldValue is "color: red; height: 100px;"
+...mutation record created.
+PASS mutations is null
+
+Testing that a no-op attiuteStyleMap property mutation does not create Mutation Records.
+PASS mutations is null
+
 Test that mutating an attribute through an attr node delivers mutation records
 PASS mutations.length is 1
 PASS mutations[0].target is div

diff --git a/LayoutTests/fast/dom/MutationObserver/observe-attributes.html b/LayoutTests/fast/dom/MutationObserver/observe-attributes.html
@@ -769,6 +769,141 @@
     start();
 }
 
+function testAttributeStyleMapAccess() {
+    var div, path;
+    var observer;
+
+    function start() {
+        debug('Testing that modifying an elements attiuteStyleMap property dispatches Mutation Records.');
+
+        mutations = null;
+        observer = new MutationObserver(function(m) {
+            mutations = m;
+        });
+
+        div = document.createElement('div');
+        div.setAttribute('style', 'color: yellow; height: 200px;');
+        observer.observe(div, { attributes: true });
+        div.attributeStyleMap.set('color', CSSStyleValue.parse('color', 'red'));
+        div.attributeStyleMap.set('height', CSS.px(100));
+        div.attributeStyleMap.set('color', CSSStyleValue.parse('color', 'blue'));
+
+        setTimeout(checkAndContinue, 0);
+    }
+
+    function checkAndContinue() {
+        shouldBe('mutations.length', '3');
+        shouldBe('mutations[0].type', '"attributes"');
+        shouldBe('mutations[0].attributeName', '"style"');
+        shouldBe('mutations[0].oldValue', 'null');
+        shouldBe('mutations[1].type', '"attributes"');
+        shouldBe('mutations[1].attributeName', '"style"');
+        shouldBe('mutations[1].oldValue', 'null');
+        shouldBe('mutations[2].type', '"attributes"');
+        shouldBe('mutations[2].attributeName', '"style"');
+        shouldBe('mutations[2].oldValue', 'null');
+
+        mutations = null;
+        div.getAttribute('style');
+        setTimeout(finish, 0);
+    }
+
+    function finish() {
+        debug('...mutation record created.');
+
+        shouldBe('mutations', 'null');
+
+        observer.disconnect();
+        debug('');
+        runNextTest();
+    }
+
+    start();
+}
+
+function testAttributeStyleMapAccessOldValue() {
+    var div, path;
+    var observer;
+
+    function start() {
+        debug('Testing that modifying an elements attiuteStyleMap property dispatches Mutation Records with correct oldValues.');
+
+        mutations = null;
+        observer = new MutationObserver(function(m) {
+            mutations = m;
+        });
+
+        div = document.createElement('div');
+        div.setAttribute('style', 'color: yellow; height: 200px;');
+        observer.observe(div, { attributes: true, attributeOldValue: true });
+        div.attributeStyleMap.set('color', CSSStyleValue.parse('color', 'red'));
+        div.attributeStyleMap.set('height', CSS.px(100));
+        div.attributeStyleMap.set('color', CSSStyleValue.parse('color', 'blue'));
+
+        setTimeout(checkAndContinue, 0);
+    }
+
+    function checkAndContinue() {
+        shouldBe('mutations.length', '3');
+        shouldBe('mutations[0].type', '"attributes"');
+        shouldBe('mutations[0].attributeName', '"style"');
+        shouldBe('mutations[0].oldValue', '"color: yellow; height: 200px;"');
+        shouldBe('mutations[1].type', '"attributes"');
+        shouldBe('mutations[1].attributeName', '"style"');
+        shouldBe('mutations[1].oldValue', '"color: red; height: 200px;"');
+        shouldBe('mutations[2].type', '"attributes"');
+        shouldBe('mutations[2].attributeName', '"style"');
+        shouldBe('mutations[2].oldValue', '"color: red; height: 100px;"');
+
+        mutations = null;
+        div.getAttribute('style');
+        setTimeout(finish, 0);
+    }
+
+    function finish() {
+        debug('...mutation record created.');
+
+        shouldBe('mutations', 'null');
+
+        observer.disconnect();
+        debug('');
+        runNextTest();
+    }
+
+    start();
+}
+
+function testAttributeStyleMapAccessIgnoreNoop() {
+    var div, path;
+    var observer;
+
+    function start() {
+        debug('Testing that a no-op attiuteStyleMap property mutation does not create Mutation Records.');
+
+        mutations = null;
+        observer = new MutationObserver(function(m) {
+            mutations = m;
+        });
+
+        div = document.createElement('div');
+        div.setAttribute('style', 'color: yellow; height: 100px;');
+        observer.observe(div, { attributes: true });
+        div.attributeStyleMap.delete('width');
+
+        setTimeout(finish, 0);
+    }
+
+    function finish() {
+        shouldBe('mutations', 'null');
+
+        observer.disconnect();
+        debug('');
+        runNextTest();
+    }
+
+    start();
+}
+
 function testMutateThroughAttrNodeValue() {
     var observer;
 
@@ -948,6 +1083,9 @@
     testStyleAttributePropertyAccess,
     testStyleAttributePropertyAccessOldValue,
     testStyleAttributePropertyAccessIgnoreNoop,
+    testAttributeStyleMapAccess,
+    testAttributeStyleMapAccessOldValue,
+    testAttributeStyleMapAccessIgnoreNoop,
     testMutateThroughAttrNodeValue,
     testSetAndRemoveAttributeNode,
     testMixedNodeAndElementOperations,

diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
@@ -939,6 +939,7 @@ css/SVGCSSComputedStyleDeclaration.cpp
 css/SelectorChecker.cpp
 css/SelectorFilter.cpp
 css/ShorthandSerializer.cpp
+css/StyleAttributeMutationScope.cpp
 css/StyleColor.cpp
 css/StyleMedia.cpp
 css/StyleProperties.cpp

diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
@@ -15530,6 +15530,8 @@
 		9B91DCCC2383792D000EEE0F /* EventLoop.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = EventLoop.cpp; sourceTree = "<group>"; };
 		9B9299B01F6796A4006723C2 /* WebContentReaderCocoa.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WebContentReaderCocoa.mm; sourceTree = "<group>"; };
 		9B9CADA72165DC7600E8D858 /* JSMutationRecordCustom.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = JSMutationRecordCustom.cpp; sourceTree = "<group>"; };
+		9B9DA7092BB5393000B8D6DB /* StyleAttributeMutationScope.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = StyleAttributeMutationScope.h; sourceTree = "<group>"; };
+		9B9DA70A2BB53D6600B8D6DB /* StyleAttributeMutationScope.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = StyleAttributeMutationScope.cpp; sourceTree = "<group>"; };
 		9BA273F3172206BB0097CE47 /* LogicalSelectionOffsetCaches.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = LogicalSelectionOffsetCaches.h; sourceTree = "<group>"; };
 		9BA827781F06156500F71E75 /* NavigationDisabler.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NavigationDisabler.h; sourceTree = "<group>"; };
 		9BAAC4562151E39E003D4A98 /* GCReachableRef.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = GCReachableRef.h; sourceTree = "<group>"; };
@@ -36125,6 +36127,8 @@
 				43107BE118CC19DE00CC18E8 /* SelectorPseudoTypeMap.h */,
 				930A76C1297FA0470055B743 /* ShorthandSerializer.cpp */,
 				930A76C2297FA04A0055B743 /* ShorthandSerializer.h */,
+				9B9DA70A2BB53D6600B8D6DB /* StyleAttributeMutationScope.cpp */,
+				9B9DA7092BB5393000B8D6DB /* StyleAttributeMutationScope.h */,
 				941827881D8B242200492764 /* StyleColor.cpp */,
 				941827891D8B242200492764 /* StyleColor.h */,
 				0FF5026E102BA9660066F39A /* StyleMedia.cpp */,