cssText setter should change style attribute when the serialization d…

…iffers https://bugs.webkit.org/show_bug.cgi?id=166716 rdar://29861252 Reviewed by Antti Koivisto. 173663@main regressed our behavior with regards to the standard. This attempts to correct it while preserving as much of the optimization as possible. WPT css/cssom is synchronized up to and including: web-platform-tests/wpt#45019 * LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-invalidation-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-invalidation.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window-expected.txt: Added. * LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window.js: Added. (document.createElement.string_appeared_here.forEach.element.cssText.setter.should.set style): * LayoutTests/imported/w3c/web-platform-tests/css/cssom/w3c-import.log: * Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp: (WebCore::PropertySetCSSStyleDeclaration::setCssText): (WebCore::PropertySetCSSStyleDeclaration::setProperty): (WebCore::PropertySetCSSStyleDeclaration::removeProperty): (WebCore::PropertySetCSSStyleDeclaration::setPropertyInternal): (WebCore::StyleRuleCSSStyleDeclaration::didMutate): (WebCore::InlineCSSStyleDeclaration::didMutate): * Source/WebCore/css/PropertySetCSSStyleDeclaration.h: * Source/WebCore/dom/StyledElement.cpp: (WebCore::StyledElement::dirtyStyleAttribute): * Source/WebCore/dom/StyledElement.h: * Source/WebCore/style/StyleScopeRuleSets.cpp: (WebCore::Style::ScopeRuleSets::hasSimpleSelectorsForStyleAttribute const): * Source/WebCore/style/StyleScopeRuleSets.h: Canonical link: https://commits.webkit.org/276176@main
WebKit · Mar 15, 2024 · 9494ab0 · 9494ab0
1 parent 890ebde
commit 9494ab0
Show file tree

Hide file tree

Showing 12 changed files with 167 additions and 9 deletions.
diff --git a/...ed/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-invalidation-expected.txt b/...ed/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-invalidation-expected.txt
@@ -0,0 +1,6 @@
+Should be green. Should be green.
+
+PASS mutating constructed CSSStyleSheet applied to root invalidates styles
+PASS mutating constructed CSSStyleSheet applied to shadowdom invalidates styles
+PASS mutating dependent constructed CSSStyleSheet applied to shadowdom invalidates styles
+
diff --git a/...s/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-invalidation.html b/...s/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-invalidation.html
@@ -0,0 +1,49 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSSStyleSheet rule mutation invalidation</title>
+<link rel="author" href="mailto:wpt@keithcirkel.co.uk" title="Keith Cirkel">
+<link rel="help" href="https://drafts.csswg.org/cssom/#extensions-to-the-document-or-shadow-root-interface">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<span id="span1">Should be green.</span>
+<span id="span2">Should be green.</span>
+<script>
+promise_test(async function(t) {
+  const sheet = new CSSStyleSheet();
+  sheet.replaceSync('span {color:var(--color, red);}');
+  document.adoptedStyleSheets = [sheet];
+  t.add_cleanup(() => {
+    document.adoptedStyleSheets = [];
+  })
+  assert_equals(getComputedStyle(span1).color, "rgb(255, 0, 0)", "Sheet should apply");
+  sheet.rules[0].style.setProperty('--color', 'green');
+  assert_equals(getComputedStyle(span1).color, "rgb(0, 128, 0)", "Sheet should invalidate style");
+  document.adoptedStyleSheets = [];
+  assert_equals(getComputedStyle(span1).color, "rgb(0, 0, 0)", "Removing sheet should apply");
+}, "mutating constructed CSSStyleSheet applied to root invalidates styles");
+
+promise_test(async function() {
+  span1.attachShadow({mode:'open'})
+  span1.shadowRoot.append(document.createElement('slot'))
+  const sheet = new CSSStyleSheet();
+  sheet.replaceSync(':host {color:var(--color, red);}');
+  span1.shadowRoot.adoptedStyleSheets = [sheet];
+  assert_equals(getComputedStyle(span1).color, "rgb(255, 0, 0)", "Sheet should apply");
+  sheet.rules[0].style.setProperty('--color', 'green');
+  assert_equals(getComputedStyle(span1).color, "rgb(0, 128, 0)", "Sheet should invalidate style");
+}, "mutating constructed CSSStyleSheet applied to shadowdom invalidates styles");
+
+promise_test(async function() {
+  span2.attachShadow({mode:'open'})
+  span2.shadowRoot.append(document.createElement('slot'))
+  const sheet1 = new CSSStyleSheet();
+  const sheet2 = new CSSStyleSheet();
+  sheet1.replaceSync(':host {color:var(--color, hotpink);}');
+  sheet2.replaceSync(':host {--color: blue}');
+  const style2 = sheet2.rules[0].style;
+  span2.shadowRoot.adoptedStyleSheets = [sheet1, sheet2];
+  assert_equals(getComputedStyle(span2).color, "rgb(0, 0, 255)", "Sheet should apply");
+  style2.setProperty('--color', 'green');
+  assert_equals(getComputedStyle(span2).color, "rgb(0, 128, 0)", "Sheet should invalidate style");
+}, "mutating dependent constructed CSSStyleSheet applied to shadowdom invalidates styles");
+</script>
diff --git a/...d/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window-expected.txt b/...d/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window-expected.txt
@@ -0,0 +1,11 @@
+
+PASS cssText setter should set style attribute even when there are no style changes (body)
+PASS cssText setter should set style attribute even when there are no style changes, part 2 (body)
+PASS cssText setter should set style attribute even when there are no style changes, part 3 (body)
+PASS cssText setter should set style attribute even when there are no style changes, part 4 (body)
+PASS cssText setter should set style attribute even when there are no style changes (cool-beans)
+PASS cssText setter should set style attribute even when there are no style changes, part 2 (cool-beans)
+PASS cssText setter should set style attribute even when there are no style changes, part 3 (cool-beans)
+PASS cssText setter should set style attribute even when there are no style changes, part 4 (cool-beans)
+PASS cssText setter and selector invalidation
+
diff --git a/.../imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window.html b/.../imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window.html
@@ -0,0 +1 @@
+<!-- This file is required for WebKit test infrastructure to run the templated test -->
diff --git a/...ts/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window.js b/...ts/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window.js
@@ -0,0 +1,64 @@
+// https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-csstext
+
+[
+  document.body,
+  document.createElement("cool-beans")
+].forEach(element => {
+  test(t => {
+    t.add_cleanup(() => element.removeAttribute("style"));
+
+    element.style.background = "red";
+    assert_equals(element.getAttribute("style"), "background: red;");
+
+    element.style.cssText = "background:red";
+    assert_equals(element.getAttribute("style"), "background: red;");
+  }, `cssText setter should set style attribute even when there are no style changes (${element.localName})`);
+
+  test(t => {
+    t.add_cleanup(() => element.removeAttribute("style"));
+
+    element.setAttribute("style", "background:   red");
+    assert_equals(element.getAttribute("style"), "background:   red");
+
+    element.style.cssText = "background:red";
+    assert_equals(element.getAttribute("style"), "background: red;");
+  }, `cssText setter should set style attribute even when there are no style changes, part 2 (${element.localName})`);
+
+  test(t => {
+    t.add_cleanup(() => element.removeAttribute("style"));
+
+    element.setAttribute("style", "background:   red");
+    assert_equals(element.getAttribute("style"), "background:   red");
+
+    element.style.cssText = "background:red "; // trailing space
+    assert_equals(element.getAttribute("style"), "background: red;");
+  }, `cssText setter should set style attribute even when there are no style changes, part 3 (${element.localName})`);
+
+  test(t => {
+    t.add_cleanup(() => element.removeAttribute("style"));
+
+    element.setAttribute("style", "background:   red");
+    assert_equals(element.getAttribute("style"), "background:   red");
+
+    element.style.cssText = "background:red;";
+    assert_equals(element.getAttribute("style"), "background: red;");
+  }, `cssText setter should set style attribute even when there are no style changes, part 4 (${element.localName})`);
+});
+
+test(t => {
+  const style = document.createElement("style");
+  t.add_cleanup(() => {
+    document.body.removeAttribute("style");
+    style.remove();
+  });
+  style.textContent = `[style="background: red;"] { background:white !important; }`;
+  document.body.appendChild(style);
+
+  document.body.setAttribute("style", "background:red");
+  assert_true(document.body.matches("[style=\"background:red\"]"));
+  assert_equals(getComputedStyle(document.body).backgroundColor, "rgb(255, 0, 0)");
+
+  document.body.style.cssText = "background:red";
+  assert_equals(getComputedStyle(document.body).backgroundColor, "rgb(255, 255, 255)");
+  assert_true(document.body.matches("[style=\"background: red;\"]"));
+}, `cssText setter and selector invalidation`);
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/cssom/w3c-import.log b/LayoutTests/imported/w3c/web-platform-tests/css/cssom/w3c-import.log
@@ -37,6 +37,7 @@ List of files:
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-disabled-regular-sheet-insertion.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-disallow-import.tentative.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-duplicate.html
+/LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-invalidation.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-replace-cssRules.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable-replace-on-regular-sheet.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/CSSStyleSheet-constructable.html
@@ -93,6 +94,7 @@ List of files:
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-all-shorthand.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-final-delimiter.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-important.html
+/LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext-setter.window.js
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-csstext.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-custom-properties.html
 /LayoutTests/imported/w3c/web-platform-tests/css/cssom/cssstyledeclaration-mutability.html

diff --git a/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp b/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp
@@ -187,8 +187,7 @@ ExceptionOr<void> PropertySetCSSStyleDeclaration::setCssText(const String& text)
         return { };
 
     bool changed = m_propertySet->parseDeclaration(text, cssParserContext());
-
-    didMutate(changed ? PropertyChanged : NoChanges);
+    didMutate(changed ? MutationType::PropertyChanged : MutationType::StyleAttributeChanged);
 
     mutationScope.enqueueMutationRecord();
     return { };
@@ -268,7 +267,7 @@ ExceptionOr<void> PropertySetCSSStyleDeclaration::setProperty(const String& prop
     else
         changed = m_propertySet->setProperty(propertyID, value, important, cssParserContext());
 
-    didMutate(changed ? PropertyChanged : NoChanges);
+    didMutate(changed ? MutationType::PropertyChanged : MutationType::NoChanges);
 
     if (changed) {
         // CSS DOM requires raising SyntaxError of parsing failed, but this is too dangerous for compatibility,
@@ -294,7 +293,7 @@ ExceptionOr<String> PropertySetCSSStyleDeclaration::removeProperty(const String&
     String result;
     bool changed = propertyID != CSSPropertyCustom ? m_propertySet->removeProperty(propertyID, &result) : m_propertySet->removeCustomProperty(propertyName, &result);
 
-    didMutate(changed ? PropertyChanged : NoChanges);
+    didMutate(changed ? MutationType::PropertyChanged : MutationType::NoChanges);
 
     if (changed)
         mutationScope.enqueueMutationRecord();
@@ -324,10 +323,10 @@ ExceptionOr<void> PropertySetCSSStyleDeclaration::setPropertyInternal(CSSPropert
         return { };
 
     if (m_propertySet->setProperty(propertyID, value, important, cssParserContext())) {
-        didMutate(PropertyChanged);
+        didMutate(MutationType::PropertyChanged);
         mutationScope.enqueueMutationRecord();
     } else
-        didMutate(NoChanges);
+        didMutate(MutationType::NoChanges);
 
     return { };
 }
@@ -402,7 +401,7 @@ void StyleRuleCSSStyleDeclaration::didMutate(MutationType type)
     ASSERT(m_parentRule);
     ASSERT(m_parentRule->parentStyleSheet());
 
-    if (type == PropertyChanged)
+    if (type == MutationType::PropertyChanged)
         m_cssomValueWrappers.clear();
 
     // Style sheet mutation needs to be signaled even if the change failed. willMutate*/didMutate* must pair.
@@ -442,8 +441,13 @@ bool InlineCSSStyleDeclaration::willMutate()
 
 void InlineCSSStyleDeclaration::didMutate(MutationType type)
 {
-    if (type == NoChanges)
+    if (type == MutationType::NoChanges)
+        return;
+
+    if (type == MutationType::StyleAttributeChanged && m_parentElement) {
+        m_parentElement->dirtyStyleAttribute();
         return;
+    }
 
     m_cssomValueWrappers.clear();
 

diff --git a/Source/WebCore/css/PropertySetCSSStyleDeclaration.h b/Source/WebCore/css/PropertySetCSSStyleDeclaration.h
@@ -53,7 +53,7 @@ class PropertySetCSSStyleDeclaration : public CSSStyleDeclaration {
     StyleSheetContents* contextStyleSheet() const;
 
 protected:
-    enum MutationType { NoChanges, PropertyChanged };
+    enum class MutationType : uint8_t { NoChanges, StyleAttributeChanged, PropertyChanged };
 
     virtual CSSParserContext cssParserContext() const;
 

diff --git a/Source/WebCore/dom/StyledElement.cpp b/Source/WebCore/dom/StyledElement.cpp
@@ -162,6 +162,20 @@ void StyledElement::styleAttributeChanged(const AtomString& newStyleString, Attr
     InspectorInstrumentation::didInvalidateStyleAttr(*this);
 }
 
+void StyledElement::dirtyStyleAttribute()
+{
+    elementData()->setStyleAttributeIsDirty(true);
+
+    if (styleResolver().ruleSets().hasSelectorsForStyleAttribute()) {
+        if (auto* inlineStyle = this->inlineStyle()) {
+            elementData()->setStyleAttributeIsDirty(false);
+            auto newValue = inlineStyle->asTextAtom();
+            Style::AttributeChangeInvalidation styleInvalidation(*this, styleAttr, attributeWithoutSynchronization(styleAttr), newValue);
+            setSynchronizedLazyAttribute(styleAttr, newValue);
+        }
+    }
+}
+
 void StyledElement::invalidateStyleAttribute()
 {
     if (auto* inlineStyle = this->inlineStyle()) {

diff --git a/Source/WebCore/dom/StyledElement.h b/Source/WebCore/dom/StyledElement.h
@@ -44,6 +44,7 @@ class StyledElement : public Element {
 public:
     virtual ~StyledElement();
 
+    void dirtyStyleAttribute();
     void invalidateStyleAttribute();
 
     const StyleProperties* inlineStyle() const { return elementData() ? elementData()->m_inlineStyle.get() : nullptr; }

diff --git a/Source/WebCore/style/StyleScopeRuleSets.cpp b/Source/WebCore/style/StyleScopeRuleSets.cpp
@@ -370,6 +370,11 @@ const HashSet<AtomString>& ScopeRuleSets::customPropertyNamesInStyleContainerQue
     return *m_customPropertyNamesInStyleContainerQueries;
 }
 
+bool ScopeRuleSets::hasSelectorsForStyleAttribute() const
+{
+    return !!attributeInvalidationRuleSets(HTMLNames::styleAttr->localName());
+}
+
 bool ScopeRuleSets::hasComplexSelectorsForStyleAttribute() const
 {
     auto compute = [&] {

diff --git a/Source/WebCore/style/StyleScopeRuleSets.h b/Source/WebCore/style/StyleScopeRuleSets.h
@@ -78,6 +78,7 @@ class ScopeRuleSets {
 
     const HashSet<AtomString>& customPropertyNamesInStyleContainerQueries() const;
 
+    bool hasSelectorsForStyleAttribute() const;
     bool hasComplexSelectorsForStyleAttribute() const;
 
     void setUsesSharedUserStyle(bool b) { m_usesSharedUserStyle = b; }