Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix style invalidation of IDs within :nth-child/:nth-last-child
https://bugs.webkit.org/show_bug.cgi?id=257848
rdar://110451692

Reviewed by Antti Koivisto.

When IDs change within :nth-child / :nth-last-child, we have to use ruleset invalidation, for this:

1. we collect the ID features when MatchElement is AnySibling in RuleFeatureSet (:nth-child/last-child selectors)
2. we store the match element rulesets on IdChangeInvalidation, since we'll want to run ruleset invalidation both
before & after (ID changes can affect siblings even when the new ID is not in the CSS)

* LayoutTests/TestExpectations:
* Source/WebCore/style/IdChangeInvalidation.cpp:
(WebCore::Style::IdChangeInvalidation::invalidateStyle):
(WebCore::Style::IdChangeInvalidation::invalidateStyleWithRuleSets):
* Source/WebCore/style/IdChangeInvalidation.h:
(WebCore::Style::IdChangeInvalidation::IdChangeInvalidation):
(WebCore::Style::IdChangeInvalidation::~IdChangeInvalidation):
* Source/WebCore/style/RuleFeature.cpp:
(WebCore::Style::RuleFeatureSet::recursivelyCollectFeaturesFromSelector):

Canonical link: https://commits.webkit.org/264986@main
  • Loading branch information
nt1m committed Jun 8, 2023
1 parent 4ca184a commit 3275687
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 8 deletions.
2 changes: 0 additions & 2 deletions LayoutTests/TestExpectations
Expand Up @@ -1656,9 +1656,7 @@ imported/w3c/web-platform-tests/css/selectors/focus-within-004.html [ ImageOnlyF
# :nth-child(n of S) invalidation
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-ids.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/selectors/invalidation/nth-last-child-of-has.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/selectors/invalidation/nth-last-child-of-ids.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/selectors/invalidation/nth-last-child-of-in-ancestor.html [ ImageOnlyFailure ]

# This test is bit heavy for debug
Expand Down
12 changes: 7 additions & 5 deletions Source/WebCore/style/IdChangeInvalidation.cpp
Expand Up @@ -29,7 +29,6 @@
#include "ElementChildIteratorInlines.h"
#include "ElementRareData.h"
#include "StyleInvalidationFunctions.h"
#include "StyleInvalidator.h"

namespace WebCore {
namespace Style {
Expand Down Expand Up @@ -69,14 +68,17 @@ void IdChangeInvalidation::invalidateStyle(const AtomString& changedId)
else
m_element.invalidateStyle();

// Invalidation rulesets exist for :has().
// Invalidation rulesets exist for :has() / :nth-child() / :nth-last-child.
if (auto* invalidationRuleSets = ruleSets.idInvalidationRuleSets(changedId)) {
Invalidator::MatchElementRuleSets matchElementRuleSets;
for (auto& invalidationRuleSet : *invalidationRuleSets)
Invalidator::addToMatchElementRuleSets(matchElementRuleSets, invalidationRuleSet);
Invalidator::invalidateWithMatchElementRuleSets(m_element, matchElementRuleSets);
Invalidator::addToMatchElementRuleSets(m_matchElementRuleSets, invalidationRuleSet);
}
}

void IdChangeInvalidation::invalidateStyleWithRuleSets()
{
Invalidator::invalidateWithMatchElementRuleSets(m_element, m_matchElementRuleSets);
}

}
}
7 changes: 7 additions & 0 deletions Source/WebCore/style/IdChangeInvalidation.h
Expand Up @@ -26,6 +26,7 @@
#pragma once

#include "Element.h"
#include "StyleInvalidator.h"

namespace WebCore {

Expand All @@ -38,11 +39,14 @@ class IdChangeInvalidation {

private:
void invalidateStyle(const AtomString&);
void invalidateStyleWithRuleSets();

const bool m_isEnabled;
Element& m_element;

AtomString m_newId;

Invalidator::MatchElementRuleSets m_matchElementRuleSets;
};

inline IdChangeInvalidation::IdChangeInvalidation(Element& element, const AtomString& oldId, const AtomString& newId)
Expand All @@ -54,14 +58,17 @@ inline IdChangeInvalidation::IdChangeInvalidation(Element& element, const AtomSt
if (oldId == newId)
return;
m_newId = newId;

invalidateStyle(oldId);
invalidateStyleWithRuleSets();
}

inline IdChangeInvalidation::~IdChangeInvalidation()
{
if (!m_isEnabled)
return;
invalidateStyle(m_newId);
invalidateStyleWithRuleSets();
}

}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/style/RuleFeature.cpp
Expand Up @@ -224,7 +224,7 @@ void RuleFeatureSet::recursivelyCollectFeaturesFromSelector(SelectorFeatures& se
idsInRules.add(selector->value());
if (matchElement == MatchElement::Parent || matchElement == MatchElement::Ancestor)
idsMatchingAncestorsInRules.add(selector->value());
else if (isHasPseudoClassMatchElement(matchElement))
else if (isHasPseudoClassMatchElement(matchElement) || matchElement == MatchElement::AnySibling)
selectorFeatures.ids.append({ selector, matchElement, isNegation });
} else if (selector->match() == CSSSelector::Class)
selectorFeatures.classes.append({ selector, matchElement, isNegation });
Expand Down

0 comments on commit 3275687

Please sign in to comment.