Fix invalidation for class names within :nth-child() selector lists

nt1m · nt1m · commit f2bf58888518 · 2023-01-14T10:27:43.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=250551 rdar://104241960 Reviewed by Darin Adler. There is an optimization in ClassChangeInvalidation that assumes only adding a class makes new elements match. However, it is not true in the case of :nth-child(n of .class), removing the class name can make new elements match too. Special case MatchElement::AnySibling (used by :nth-child() invalidation) to reflect this. Tests: - imported/w3c/web-platform-tests/css/selectors/invalidation/nth-child-of-class.html - imported/w3c/web-platform-tests/css/selectors/invalidation/nth-child-of-sibling.html * LayoutTests/TestExpectations: * Source/WebCore/style/ClassChangeInvalidation.cpp: (WebCore::Style::ClassChangeInvalidation::computeInvalidation): Canonical link: https://commits.webkit.org/258917@main
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -1636,10 +1636,8 @@ webkit.org/b/238822 imported/w3c/web-platform-tests/css/selectors/child-indexed-
 webkit.org/b/238822 imported/w3c/web-platform-tests/css/selectors/selector-structural-pseudo-root.html [ ImageOnlyFailure ]
 
 # :nth-child(n of S) invalidation
-imported/w3c/web-platform-tests/css/selectors/invalidation/nth-child-of-class.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/selectors/invalidation/nth-child-of-has.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/selectors/invalidation/nth-child-of-in-ancestor.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/selectors/invalidation/nth-child-of-sibling.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/selectors/invalidation/nth-child-in-shadow-root.html [ ImageOnlyFailure ]
 
 # This test is bit heavy for debug
diff --git a/Source/WebCore/style/ClassChangeInvalidation.cpp b/Source/WebCore/style/ClassChangeInvalidation.cpp
@@ -116,18 +116,24 @@ void ClassChangeInvalidation::computeInvalidation(const SpaceSplitString& oldCla
 
     auto& ruleSets = m_element.styleResolver().ruleSets();
 
-    auto invalidateBeforeChange = [](ClassChangeType type, IsNegation isNegation) {
-        if (type == ClassChangeType::Remove)
-            return isNegation == IsNegation::No;
-        return isNegation == IsNegation::Yes;
+    auto invalidateBeforeChange = [](ClassChangeType type, IsNegation isNegation, MatchElement matchElement) {
+        if (matchElement == MatchElement::AnySibling)
+            return true;
+        return type == ClassChangeType::Remove ? isNegation == IsNegation::No : isNegation == IsNegation::Yes;
+    };
+
+    auto invalidateAfterChange = [](ClassChangeType type, IsNegation isNegation, MatchElement matchElement) {
+        if (matchElement == MatchElement::AnySibling)
+            return true;
+        return type == ClassChangeType::Add ? isNegation == IsNegation::No : isNegation == IsNegation::Yes;
     };
 
     for (auto& classChange : classChanges) {
         if (auto* invalidationRuleSets = ruleSets.classInvalidationRuleSets(classChange.className)) {
             for (auto& invalidationRuleSet : *invalidationRuleSets) {
-                if (invalidateBeforeChange(classChange.type, invalidationRuleSet.isNegation))
+                if (invalidateBeforeChange(classChange.type, invalidationRuleSet.isNegation, invalidationRuleSet.matchElement))
                     Invalidator::addToMatchElementRuleSets(m_beforeChangeRuleSets, invalidationRuleSet);
-                else
+                if (invalidateAfterChange(classChange.type, invalidationRuleSet.isNegation, invalidationRuleSet.matchElement))
                     Invalidator::addToMatchElementRuleSets(m_afterChangeRuleSets, invalidationRuleSet);
             }
         }
