Browse Source

Traverse the style sheet nodes recursively to test all possible paths toward the root, not just the first greedy path.

Add unit tests which passes with this change.
Michael Ragazzon 3 years ago
parent
commit
e6aea9369a
3 changed files with 73 additions and 47 deletions
  1. 55 47
      Source/Core/StyleSheetNode.cpp
  2. 3 0
      Source/Core/StyleSheetNode.h
  3. 15 0
      Tests/Source/UnitTests/Selectors.cpp

+ 55 - 47
Source/Core/StyleSheetNode.cpp

@@ -256,75 +256,83 @@ inline bool StyleSheetNode::MatchStructuralSelector(const Element* element) cons
 	return true;
 }
 
-bool StyleSheetNode::IsApplicable(const Element* const in_element) const
+bool StyleSheetNode::TraverseMatch(const StyleSheetNode* node, const Element* element)
+{
+	if (!node || !node->parent)
+		return true;
+
+	switch (node->combinator)
+	{
+	case SelectorCombinator::None:
+	case SelectorCombinator::Child:
+	{
+		// Try to match the next element parent. If it succeeds we continue on to the next node, otherwise we try an alternate path through the
+		// hierarchy using the next element parent. Repeat until we run out of elements.
+		for (element = element->GetParentNode(); element; element = element->GetParentNode())
+		{
+			if (node->Match(element) && TraverseMatch(node->parent, element))
+				return true;
+			// If the node has a child combinator we must match this first ancestor.
+			else if (node->combinator == SelectorCombinator::Child)
+				return false;
+		}
+	}
+	break;
+	case SelectorCombinator::NextSibling:
+	case SelectorCombinator::SubsequentSibling:
+	{
+		// Try to match the previous sibling. If it succeeds we continue on to the next node, otherwise we try to again with its previous sibling.
+		for (element = element->GetPreviousSibling(); element; element = element->GetPreviousSibling())
+		{
+			// First check if our sibling is a text element and if so skip it. For the descendant/child combinator above we can omit this step since
+			// text elements don't have children and thus any ancestor is not a text element.
+			if (IsTextElement(element))
+				continue;
+			else if (node->Match(element) && TraverseMatch(node->parent, element))
+				return true;
+			// If the node has a next-sibling combinator we must match this first sibling.
+			else if (node->combinator == SelectorCombinator::NextSibling)
+				return false;
+		}
+	}
+	break;
+	}
+
+	// We have run out of element ancestors before we matched every node. Bail out.
+	return false;
+}
+
+bool StyleSheetNode::IsApplicable(const Element* element) const
 {
 	// Determine whether the element matches the current node and its entire lineage. The entire hierarchy of the element's document will be
 	// considered during the match as necessary.
 
 	// We could in principle just call Match() here and then go on with the ancestor style nodes. Instead, we test the requirements of this node in a
-	// particular order for performance reasons .
+	// particular order for performance reasons.
 	for (const String& name : pseudo_class_names)
 	{
-		if (!in_element->IsPseudoClassSet(name))
+		if (!element->IsPseudoClassSet(name))
 			return false;
 	}
 
-	if (!tag.empty() && tag != in_element->GetTagName())
+	if (!tag.empty() && tag != element->GetTagName())
 		return false;
 
 	for (const String& name : class_names)
 	{
-		if (!in_element->IsClassSet(name))
+		if (!element->IsClassSet(name))
 			return false;
 	}
 
-	if (!id.empty() && id != in_element->GetId())
+	if (!id.empty() && id != element->GetId())
 		return false;
 
-	const Element* element = in_element;
-
 	// Walk up through all our parent nodes, each one of them must be matched by some ancestor or sibling element.
-	for (const StyleSheetNode* node = parent; node && node->parent; node = node->parent)
-	{
-		switch (node->combinator)
-		{
-		case SelectorCombinator::None:
-		case SelectorCombinator::Child:
-		{
-			// Try a match on every element ancestor. If it succeeds, we continue on to the next node.
-			for (element = element->GetParentNode(); element; element = element->GetParentNode())
-			{
-				if (node->Match(element))
-					break;
-				// If the node has a child combinator we must match this first ancestor.
-				else if (node->combinator == SelectorCombinator::Child)
-					return false;
-			}
-		}
-		break;
-		case SelectorCombinator::NextSibling:
-		case SelectorCombinator::SubsequentSibling:
-		{
-			// Try a match on every element ancestor. If it succeeds, we continue on to the next node.
-			for (element = element->GetPreviousSibling(); element; element = element->GetPreviousSibling())
-			{
-				if (node->Match(element) && !IsTextElement(in_element))
-					break;
-				// If the node has a next-sibling combinator we must match this first sibling.
-				else if (node->combinator == SelectorCombinator::NextSibling && !IsTextElement(in_element))
-					return false;
-			}
-		}
-		break;
-		}
-
-		// We have run out of element ancestors before we matched every node. Bail out.
-		if (!element)
-			return false;
-	}
+	if (!TraverseMatch(parent, element))
+		return false;
 
 	// Finally, check the structural selector requirements last as they can be quite slow.
-	if (!MatchStructuralSelector(in_element))
+	if (!MatchStructuralSelector(element))
 		return false;
 
 	return true;

+ 3 - 0
Source/Core/StyleSheetNode.h

@@ -95,6 +95,9 @@ private:
 	inline bool MatchClassPseudoClass(const Element* element) const;
 	inline bool MatchStructuralSelector(const Element* element) const;
 
+	// Recursively traverse the nodes up towards the root to match the element and its hierarchy.
+	static bool TraverseMatch(const StyleSheetNode* node, const Element* element);
+
 	// The parent of this node; is nullptr for the root node.
 	StyleSheetNode* parent = nullptr;
 

+ 15 - 0
Tests/Source/UnitTests/Selectors.cpp

@@ -102,6 +102,7 @@ static const Vector<QuerySelector> query_selectors =
 	{ ".parent *",                   "A B C D D0 D1 E F F0 G H" },
 	{ ".parent > *",                 "A B C D E F G H" },
 	{ ":checked",                    "I",               SelectorOp::RemoveChecked,        "I", "" },
+	
 	{ ".parent :nth-child(odd)",     "A C D0 E F0 G" },
 	{ ".parent > :nth-child(even)",  "B D F H",         SelectorOp::RemoveClasses,        "parent", "" },
 	{ ":first-child",                "X A D0 F0",       SelectorOp::RemoveElementsByIds,  "A F0", "X B D0" },
@@ -124,16 +125,23 @@ static const Vector<QuerySelector> query_selectors =
 	{ ":only-child",                 "F0",              SelectorOp::RemoveElementsByIds,  "D0",    "D1 F0" },
 	{ ":only-of-type",               "Y A E F0 I" },
 	{ "span:empty",                  "Y D0 F0" },
+	
 	{ ".hello.world, #P span, #I",   "Z D0 D1 F0 I",    SelectorOp::RemoveClasses,        "world", "D0 D1 F0 I" },
 	{ "body * span",                 "D0 D1 F0" },
 	{ "D1 *",                        "" },
+	
 	{ "#E + #F",                     "F",               SelectorOp::InsertElementBefore,  "F",     "" },
 	{ "#E+#F",                       "F" },
 	{ "#E +#F",                      "F" },
 	{ "#E+ #F",                      "F" },
 	{ "#F + #E",                     "" },
+	{ "#A + #B",                     "B",               SelectorOp::RemoveId,             "A", "" },
+	{ "* + #A",                      "" },
+	{ "#H + *",                      "" },
+	{ "#P + *",                      "I" },
 	{ "div.parent > #B + p",         "C" },
 	{ "div.parent > #B + div",       "" },
+	
 	{ "#B ~ #F",                     "F" },
 	{ "#B~#F",                       "F" },
 	{ "#B ~#F",                      "F" },
@@ -141,18 +149,25 @@ static const Vector<QuerySelector> query_selectors =
 	{ "#F ~ #B",                     "" },
 	{ "div.parent > #B ~ p:empty",   "C G H",           SelectorOp::InsertElementBefore,  "H",     "C G Inserted H" },
 	{ "div.parent > #B ~ * span",    "D0 D1 F0" },
+
 	{ ":not(*)",                     "" },
 	{ ":not(span)",                  "X Z P A B C D E F G H I" },
 	{ "#D :not(#D0)",                "D1" },
 	{ "body > :not(:checked)",       "X Y Z P",         SelectorOp::RemoveChecked,        "I", "X Y Z P I" },
 	{ "div.hello:not(.world)",       "X" },
 	{ ":not(div,:nth-child(2),p *)", "A C D E F G H I" },
+
 	{ ".hello + .world",             "Y",               SelectorOp::RemoveClasses,        "hello", ""  },
 	{ "#B ~ h3",                     "E",               SelectorOp::RemoveId,             "B", ""  },
 	{ "#Z + div > :nth-child(2)",    "B",               SelectorOp::RemoveId,             "Z", ""  },
 	{ ":hover + #P #D1",             "",                SelectorOp::SetHover,             "Z", "D1"  },
 	{ ":not(:hover) + #P #D1",       "D1",              SelectorOp::SetHover,             "Z", ""  },
 	{ "#X + #Y",                     "Y",               SelectorOp::RemoveId,             "X", ""  },
+
+	{ "body > * #D0",                "D0" },
+	{ "#E + * ~ *",                  "G H" },
+	{ "#B + * ~ #G",                 "G" },
+	{ "body > :nth-child(4) span:first-child",  "D0 F0", SelectorOp::RemoveElementsByIds,  "X",    "" },
 };
 
 struct ClosestSelector {