We currently get order wrong in cases like @layer a.b { ... } @layer c { ... } @layer a.d { ... } where c should have higher priority than a.d.
Created attachment 436763 [details] patch
Created attachment 436764 [details] patch
Comment on attachment 436764 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=436764&action=review > Source/WebCore/ChangeLog:3 > + [CSS Cascade Layers] Computer order correctly for late added sublayers Compute order > Source/WebCore/style/RuleSet.cpp:86 > +void RuleSet::addRule(const StyleRule& rule, unsigned selectorIndex, unsigned selectorListIndex, unsigned cascadeLayerIdentifier, MediaQueryCollector* mediaQueryCollector) unsigned -> CascadeLayerIdentifier ? > Source/WebCore/style/RuleSet.cpp:93 > + std::fill(m_cascadeLayerIdentifierForRulePosition.begin() + oldSize, m_cascadeLayerIdentifierForRulePosition.end(), 0); Shame they don't init to 0 automatically > Source/WebCore/style/RuleSet.cpp:405 > + CascadeLayerIdentifier identifier = 0; Oh there is a typedef! Perhaps it should be a type which can initialize itself to 0. > Source/WebCore/style/RuleSet.cpp:470 > + auto compareCascadeLayers = [&](CascadeLayerIdentifier a, CascadeLayerIdentifier b) { > + while (a && b) { > + // Identifiers are in parse order which almost corresponds to the layer priority order. > + // The only exception is when a sublayer gets added to a layer after adding other non-sublayers. > + // To resolve this we need look for a shared ancestor layer. > + auto aParent = ruleSet->cascadeLayerForIdentifier(a).parentIdentifier; > + auto bParent = ruleSet->cascadeLayerForIdentifier(b).parentIdentifier; > + if (aParent == bParent || aParent == b || bParent == a) > + break; > + if (aParent > bParent) > + a = aParent; > + else > + b = bParent; > + } > + return a < b; > + }; > + > + auto updateCascadeLayerOrder = [&] { > + Vector<CascadeLayerIdentifier> orderVector; > + for (CascadeLayerIdentifier identifier = 1; identifier <= ruleSet->m_cascadeLayers.size(); ++identifier) > + orderVector.append(identifier); > + std::sort(orderVector.begin(), orderVector.end(), compareCascadeLayers); > + for (unsigned i = 0; i < orderVector.size(); ++i) > + ruleSet->cascadeLayerForIdentifier(orderVector[i]).order = i + 1; > + }; > + > + if (mode == Mode::Normal && !ruleSet->m_cascadeLayers.isEmpty()) > + updateCascadeLayerOrder(); Maybe put this in its own function > Source/WebCore/style/RuleSet.h:193 > + CascadeLayer& cascadeLayerForIdentifier(CascadeLayerIdentifier identifier) { return m_cascadeLayers[identifier - 1]; } > + const CascadeLayer& cascadeLayerForIdentifier(CascadeLayerIdentifier identifier) const { return m_cascadeLayers[identifier - 1]; } Maybe assert that identifier != 0
(In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 436764 [details] > Shame they don't init to 0 automatically Yeah, was going to add a variant of grow that takes an initialization value.
> > Source/WebCore/style/RuleSet.h:193 > > + CascadeLayer& cascadeLayerForIdentifier(CascadeLayerIdentifier identifier) { return m_cascadeLayers[identifier - 1]; } > > + const CascadeLayer& cascadeLayerForIdentifier(CascadeLayerIdentifier identifier) const { return m_cascadeLayers[identifier - 1]; } > > Maybe assert that identifier != 0 Didn't seem necessary since vector asserts against overflow.
Created attachment 436788 [details] patch
Created attachment 436794 [details] patch
Committed r281798 (241135@main): <https://commits.webkit.org/241135@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 436794 [details].
<rdar://problem/82574233>