Browse Source

Merge branch 'selectors'

Michael Ragazzon 3 years ago
parent
commit
54471096b0

+ 1 - 1
.clang-format

@@ -8,7 +8,7 @@ AlignEscapedNewlines: Left
 AlignOperands: false
 AlignTrailingComments: true
 AllowShortBlocksOnASingleLine: Empty
-AllowShortCaseLabelsOnASingleLine: false
+AllowShortCaseLabelsOnASingleLine: true
 AllowShortFunctionsOnASingleLine: Inline
 AlwaysBreakTemplateDeclarations: Yes
 BinPackArguments: true

+ 9 - 3
Include/RmlUi/Core/Element.h

@@ -301,6 +301,8 @@ public:
 	/// @param[in] name Name of the attribute to retrieve.
 	/// @return A variant representing the attribute, or nullptr if the attribute doesn't exist.
 	Variant* GetAttribute(const String& name);
+	/// Gets the specified attribute.
+	const Variant* GetAttribute(const String& name) const;
 	/// Gets the specified attribute, with default value.
 	/// @param[in] name Name of the attribute to retrieve.
 	/// @param[in] default_value Value to return if the attribute doesn't exist.
@@ -640,6 +642,10 @@ protected:
 	/// @param[in] activate True if the pseudo-class is to be activated, false to be deactivated.
 	static void OverridePseudoClass(Element* target_element, const String& pseudo_class, bool activate);
 
+	enum class DirtyNodes { Self, SelfAndSiblings };
+	// Dirty the element style definition, including all descendants of the specificed nodes.
+	void DirtyDefinition(DirtyNodes dirty_nodes);
+
 	void SetOwnerDocument(ElementDocument* document);
 
 	void OnStyleSheetChangeRecursive();
@@ -661,8 +667,7 @@ private:
 	static void BuildStackingContextForTable(Vector<StackingOrderedChild>& ordered_children, Element* child);
 	void DirtyStackingContext();
 
-	void DirtyStructure();
-	void UpdateStructure();
+	void UpdateDefinition();
 
 	void DirtyTransformState(bool perspective_dirty, bool transform_dirty);
 	void UpdateTransformState();
@@ -701,7 +706,8 @@ private:
 	bool offset_fixed;
 	bool absolute_offset_dirty;
 
-	bool structure_dirty : 1;
+	bool dirty_definition : 1; // Implies dirty child definitions as well.
+	bool dirty_child_definitions : 1;
 
 	bool dirty_animation : 1;
 	bool dirty_transition : 1;

+ 6 - 0
Source/Core/Context.cpp

@@ -191,6 +191,12 @@ bool Context::Update()
 	for (auto& data_model : data_models)
 		data_model.second->Update(true);
 
+	// The style definition of each document should be independent of each other. By manually resetting these flags we avoid unnecessary definition
+	// lookups in unrelated documents, such as when adding a new document. Adding an element dirties the parent definition, which in this case is the
+	// root. By extension the definition of all the other documents are also dirtied, unnecessarily.
+	root->dirty_definition = false;
+	root->dirty_child_definitions = false;
+
 	root->Update(density_independent_pixel_ratio, Vector2f(dimensions));
 
 	for (int i = 0; i < root->GetNumChildren(); ++i)

+ 52 - 28
Source/Core/Element.cpp

@@ -88,8 +88,8 @@ static Pool< ElementMeta > element_meta_chunk_pool(200, true);
 
 Element::Element(const String& tag) :
 	local_stacking_context(false), local_stacking_context_forced(false), stacking_context_dirty(false), computed_values_are_default_initialized(true),
-	visible(true), offset_fixed(false), absolute_offset_dirty(true), structure_dirty(false), dirty_animation(false), dirty_transition(false),
-	dirty_transform(false), dirty_perspective(false),
+	visible(true), offset_fixed(false), absolute_offset_dirty(true), dirty_definition(false), dirty_child_definitions(false), dirty_animation(false),
+	dirty_transition(false), dirty_transform(false), dirty_perspective(false),
 
 	tag(tag), relative_offset_base(0, 0), relative_offset_position(0, 0), absolute_offset(0, 0), scroll_offset(0, 0), content_offset(0, 0),
 	content_box(0, 0), transform_state()
@@ -145,8 +145,6 @@ void Element::Update(float dp_ratio, Vector2f vp_dimensions)
 
 	OnUpdate();
 
-	UpdateStructure();
-
 	HandleTransitionProperty();
 	HandleAnimationProperty();
 	AdvanceAnimations();
@@ -171,7 +169,7 @@ void Element::Update(float dp_ratio, Vector2f vp_dimensions)
 
 void Element::UpdateProperties(const float dp_ratio, const Vector2f vp_dimensions)
 {
-	meta->style.UpdateDefinition();
+	UpdateDefinition();
 
 	if (meta->style.AnyPropertiesDirty())
 	{
@@ -276,7 +274,8 @@ ElementPtr Element::Clone() const
 // Sets or removes a class on the element.
 void Element::SetClass(const String& class_name, bool activate)
 {
-	meta->style.SetClass(class_name, activate);
+	if (meta->style.SetClass(class_name, activate))
+		DirtyDefinition(DirtyNodes::SelfAndSiblings);
 }
 
 // Checks if a class is set on the element.
@@ -747,7 +746,11 @@ PropertiesIteratorView Element::IterateLocalProperties() const
 void Element::SetPseudoClass(const String& pseudo_class, bool activate)
 {
 	if (meta->style.SetPseudoClass(pseudo_class, activate, false))
+	{
+		// Include siblings in case of RCSS presence of sibling combinators '+', '~'.
+		DirtyDefinition(DirtyNodes::SelfAndSiblings);
 		OnPseudoClassChange(pseudo_class, activate);
+	}
 }
 
 // Checks if a specific pseudo-class has been set on the element.
@@ -788,12 +791,16 @@ void Element::OverridePseudoClass(Element* element, const String& pseudo_class,
 	element->GetStyle()->SetPseudoClass(pseudo_class, activate, true);
 }
 
-/// Get the named attribute
 Variant* Element::GetAttribute(const String& name)
 {
 	return GetIf(attributes, name);
 }
 
+const Variant* Element::GetAttribute(const String& name) const
+{
+	return GetIf(attributes, name);
+}
+
 // Checks if the element has a certain attribute.
 bool Element::HasAttribute(const String& name) const
 {
@@ -1310,7 +1317,10 @@ Element* Element::AppendChild(ElementPtr child, bool dom_element)
 		ancestor->OnChildAdd(child_ptr);
 
 	DirtyStackingContext();
-	DirtyStructure();
+
+	// Not only does the element definition of the newly inserted element need to be dirtied, but also our own definition and implicitly all of our
+	// children's. This ensures correct styles being applied in the presence of tree-structural selectors such as ':first-child'.
+	DirtyDefinition(DirtyNodes::Self);
 
 	if (dom_element)
 		DirtyLayout();
@@ -1359,7 +1369,7 @@ Element* Element::InsertBefore(ElementPtr child, Element* adjacent_element)
 			ancestor->OnChildAdd(child_ptr);
 
 		DirtyStackingContext();
-		DirtyStructure();
+		DirtyDefinition(DirtyNodes::Self);
 	}
 	else
 	{
@@ -1446,7 +1456,7 @@ ElementPtr Element::RemoveChild(Element* child)
 
 			DirtyLayout();
 			DirtyStackingContext();
-			DirtyStructure();
+			DirtyDefinition(DirtyNodes::Self);
 
 			return detached_child;
 		}
@@ -1661,7 +1671,6 @@ void Element::OnAttributeChange(const ElementAttributes& changed_attributes)
 		if (attribute == "id")
 		{
 			id = value.Get<String>();
-			meta->style.DirtyDefinition();
 		}
 		else if (attribute == "class")
 		{
@@ -1716,6 +1725,10 @@ void Element::OnAttributeChange(const ElementAttributes& changed_attributes)
 				Log::Message(Log::LT_WARNING, "Invalid 'style' attribute, string type required. In element: %s", GetAddress().c_str());
 		}
 	}
+
+	// Any change to the attributes may affect which styles apply to the current element, in particular due to attribute selectors, ID selectors, and
+	// class selectors. This can further affect all siblings or descendants due to sibling or descendant combinators.
+	DirtyDefinition(DirtyNodes::SelfAndSiblings);
 }
 
 // Called when properties on the element are changed.
@@ -1757,15 +1770,6 @@ void Element::OnPropertyChange(const PropertyIdSet& changed_properties)
 			if (!visible)
 				Blur();
 		}
-
-		if (changed_properties.Contains(PropertyId::Display))
-		{
-			// Due to structural pseudo-classes, this may change the element definition in siblings and parent.
-			// However, the definitions will only be changed on the next update loop which may result in jarring behavior for one @frame.
-			// A possible workaround is to add the parent to a list of elements that need to be updated again.
-			if (parent != nullptr)
-				parent->DirtyStructure();
-		}
 	}
 
 	// Update the position.
@@ -2075,7 +2079,7 @@ void Element::SetParent(Element* _parent)
 	if (parent)
 	{
 		// We need to update our definition and make sure we inherit the properties of our new parent.
-		meta->style.DirtyDefinition();
+		DirtyDefinition(DirtyNodes::Self);
 		meta->style.DirtyInheritedProperties();
 	}
 
@@ -2355,19 +2359,39 @@ void Element::DirtyStackingContext()
 		stacking_context_parent->stacking_context_dirty = true;
 }
 
-void Element::DirtyStructure()
+void Element::DirtyDefinition(DirtyNodes dirty_nodes)
 {
-	structure_dirty = true;
+	switch (dirty_nodes)
+	{
+	case DirtyNodes::Self:
+		dirty_definition = true;
+		break;
+	case DirtyNodes::SelfAndSiblings:
+		if (parent)
+			parent->dirty_child_definitions = true;
+		break;
+	}
 }
 
-void Element::UpdateStructure()
+void Element::UpdateDefinition()
 {
-	if (structure_dirty)
+	if (dirty_definition)
 	{
-		structure_dirty = false;
+		dirty_definition = false;
 
-		// If this element or its children depend on structured selectors, they may need to be updated.
-		GetStyle()->DirtyDefinition();
+		// Dirty definition implies all our descendent elements. Anything that can change the definition of this element can also change the
+		// definition of any descendants due to the presence of RCSS descendant or child combinators. In principle this also applies to sibling
+		// combinators, but those are handled during the DirtyDefinition call.
+		dirty_child_definitions = true;
+
+		GetStyle()->UpdateDefinition();
+	}
+
+	if (dirty_child_definitions)
+	{
+		dirty_child_definitions = false;
+		for (const ElementPtr& child : children)
+			child->dirty_definition = true;
 	}
 }
 

+ 1 - 1
Source/Core/ElementDocument.cpp

@@ -239,7 +239,7 @@ void ElementDocument::DirtyMediaQueries()
 
 		if (changed_style_sheet)
 		{
-			GetStyle()->DirtyDefinition();
+			DirtyDefinition(Element::DirtyNodes::Self);
 			OnStyleSheetChangeRecursive();
 		}
 	}

+ 38 - 61
Source/Core/ElementStyle.cpp

@@ -65,7 +65,6 @@ inline PseudoClassState operator&(PseudoClassState lhs, PseudoClassState rhs)
 ElementStyle::ElementStyle(Element* _element)
 {
 	element = _element;
-	definition_dirty = true;
 }
 
 // Returns one of this element's properties.
@@ -169,58 +168,49 @@ void ElementStyle::TransitionPropertyChanges(Element* element, PropertyIdSet& pr
 		}
 	}
 }
-	
+
 void ElementStyle::UpdateDefinition()
 {
-	if (definition_dirty)
-	{
-		RMLUI_ZoneScoped;
+	RMLUI_ZoneScoped;
 
-		definition_dirty = false;
+	SharedPtr<const ElementDefinition> new_definition;
 
-		SharedPtr<const ElementDefinition> new_definition;
-		
-		if (const StyleSheet* style_sheet = element->GetStyleSheet())
-		{
-			new_definition = style_sheet->GetElementDefinition(element);
-		}
-		
-		// Switch the property definitions if the definition has changed.
-		if (new_definition != definition)
-		{
-			PropertyIdSet changed_properties;
-			
-			if (definition)
-				changed_properties = definition->GetPropertyIds();
+	if (const StyleSheet* style_sheet = element->GetStyleSheet())
+	{
+		new_definition = style_sheet->GetElementDefinition(element);
+	}
 
-			if (new_definition)
-				changed_properties |= new_definition->GetPropertyIds();
+	// Switch the property definitions if the definition has changed.
+	if (new_definition != definition)
+	{
+		PropertyIdSet changed_properties;
 
-			if (definition && new_definition)
-			{
-				// Remove properties that compare equal from the changed list.
-				const PropertyIdSet properties_in_both_definitions = (definition->GetPropertyIds() & new_definition->GetPropertyIds());
+		if (definition)
+			changed_properties = definition->GetPropertyIds();
 
-				for (PropertyId id : properties_in_both_definitions)
-				{
-					const Property* p0 = definition->GetProperty(id);
-					const Property* p1 = new_definition->GetProperty(id);
-					if (p0 && p1 && *p0 == *p1)
-						changed_properties.Erase(id);
-				}
+		if (new_definition)
+			changed_properties |= new_definition->GetPropertyIds();
 
-				// Transition changed properties if transition property is set
-				TransitionPropertyChanges(element, changed_properties, inline_properties, definition.get(), new_definition.get());
+		if (definition && new_definition)
+		{
+			// Remove properties that compare equal from the changed list.
+			const PropertyIdSet properties_in_both_definitions = (definition->GetPropertyIds() & new_definition->GetPropertyIds());
+
+			for (PropertyId id : properties_in_both_definitions)
+			{
+				const Property* p0 = definition->GetProperty(id);
+				const Property* p1 = new_definition->GetProperty(id);
+				if (p0 && p1 && *p0 == *p1)
+					changed_properties.Erase(id);
 			}
 
-			definition = new_definition;
-			
-			DirtyProperties(changed_properties);
+			// Transition changed properties if transition property is set
+			TransitionPropertyChanges(element, changed_properties, inline_properties, definition.get(), new_definition.get());
 		}
 
-		// Even if the definition was not changed, the child definitions may have changed as a result of anything that
-		// could change the definition of this element, such as a new pseudo class.
-		DirtyChildDefinitions();
+		definition = new_definition;
+
+		DirtyProperties(changed_properties);
 	}
 }
 
@@ -250,9 +240,6 @@ bool ElementStyle::SetPseudoClass(const String& pseudo_class, bool activate, boo
 		}
 	}
 
-	if (changed)
-		DirtyDefinition();
-
 	return changed;
 }
 
@@ -267,17 +254,17 @@ const PseudoClassMap& ElementStyle::GetActivePseudoClasses() const
 	return pseudo_classes;
 }
 
-// Sets or removes a class on the element.
-void ElementStyle::SetClass(const String& class_name, bool activate)
+bool ElementStyle::SetClass(const String& class_name, bool activate)
 {
-	StringList::iterator class_location = std::find(classes.begin(), classes.end(), class_name);
+	const auto class_location = std::find(classes.begin(), classes.end(), class_name);
 
+	bool changed = false;
 	if (activate)
 	{
 		if (class_location == classes.end())
 		{
 			classes.push_back(class_name);
-			DirtyDefinition();
+			changed = true;
 		}
 	}
 	else
@@ -285,9 +272,11 @@ void ElementStyle::SetClass(const String& class_name, bool activate)
 		if (class_location != classes.end())
 		{
 			classes.erase(class_location);
-			DirtyDefinition();
+			changed = true;
 		}
 	}
+
+	return changed;
 }
 
 // Checks if a class is set on the element.
@@ -301,7 +290,6 @@ void ElementStyle::SetClassNames(const String& class_names)
 {
 	classes.clear();
 	StringUtilities::ExpandString(classes, class_names, ' ');
-	DirtyDefinition();
 }
 
 // Returns the list of classes specified for this element.
@@ -466,22 +454,11 @@ float ElementStyle::ResolveLength(const Property* property, RelativeTarget relat
 	return base_value * scale_value;
 }
 
-void ElementStyle::DirtyDefinition()
-{
-	definition_dirty = true;
-}
-
 void ElementStyle::DirtyInheritedProperties()
 {
 	dirty_properties |= StyleSheetSpecification::GetRegisteredInheritedProperties();
 }
 
-void ElementStyle::DirtyChildDefinitions()
-{
-	for (int i = 0; i < element->GetNumChildren(true); i++)
-		element->GetChild(i)->GetStyle()->DirtyDefinition();
-}
-
 void ElementStyle::DirtyPropertiesWithUnits(Property::Unit units)
 {
 	// Dirty all the properties of this element that use the unit(s).

+ 2 - 8
Source/Core/ElementStyle.h

@@ -76,7 +76,8 @@ public:
 	/// Sets or removes a class on the element.
 	/// @param[in] class_name The name of the class to add or remove from the class list.
 	/// @param[in] activate True if the class is to be added, false to be removed.
-	void SetClass(const String& class_name, bool activate);
+	/// @return True if the class was changed, false otherwise.
+	bool SetClass(const String& class_name, bool activate);
 	/// Checks if a class is set on the element.
 	/// @param[in] class_name The name of the class to check for.
 	/// @return True if the class is set on the element, false otherwise.
@@ -120,9 +121,6 @@ public:
 	/// Numbers and percentages are resolved by scaling the size of the specified target.
 	float ResolveLength(const Property* property, RelativeTarget relative_target) const;
 
-	/// Mark definition and all children dirty.
-	void DirtyDefinition();
-
 	/// Mark inherited properties dirty.
 	/// Inherited properties will automatically be set when parent inherited properties are changed. However,
 	/// some operations may require to dirty these manually, such as when moving an element into another.
@@ -147,8 +145,6 @@ public:
 	PropertiesIterator Iterate() const;
 
 private:
-	// Dirty all child definitions
-	void DirtyChildDefinitions();
 	// Sets a list of properties as dirty.
 	void DirtyProperties(const PropertyIdSet& properties);
 
@@ -168,8 +164,6 @@ private:
 	PropertyDictionary inline_properties;
 	// The definition of this element, provides applicable properties from the stylesheet.
 	SharedPtr<const ElementDefinition> definition;
-	// Set if a new element definition should be fetched from the style.
-	bool definition_dirty;
 
 	PropertyIdSet dirty_properties;
 };

+ 0 - 1
Source/Core/StyleSheet.cpp

@@ -101,7 +101,6 @@ void StyleSheet::BuildNodeIndex()
 	RMLUI_ZoneScoped;
 	styled_node_index = {};
 	root->BuildIndex(styled_node_index);
-	root->SetStructurallyVolatileRecursive(false);
 }
 
 // Returns the Keyframes of the given name, or null if it does not exist.

+ 172 - 141
Source/Core/StyleSheetNode.cpp

@@ -33,6 +33,7 @@
 #include "StyleSheetFactory.h"
 #include "StyleSheetSelector.h"
 #include <algorithm>
+#include <tuple>
 
 namespace Rml {
 
@@ -46,35 +47,27 @@ StyleSheetNode::StyleSheetNode()
 	CalculateAndSetSpecificity();
 }
 
-StyleSheetNode::StyleSheetNode(StyleSheetNode* parent, const String& tag, const String& id, const StringList& classes,
-	const StringList& pseudo_classes, const StructuralSelectorList& structural_selectors, SelectorCombinator combinator) :
-	parent(parent),
-	tag(tag), id(id), class_names(classes), pseudo_class_names(pseudo_classes), structural_selectors(structural_selectors), combinator(combinator)
+StyleSheetNode::StyleSheetNode(StyleSheetNode* parent, const CompoundSelector& selector) : parent(parent), selector(selector)
 {
 	CalculateAndSetSpecificity();
 }
 
-StyleSheetNode::StyleSheetNode(StyleSheetNode* parent, String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes,
-	StructuralSelectorList&& structural_selectors, SelectorCombinator combinator) :
-	parent(parent),
-	tag(std::move(tag)), id(std::move(id)), class_names(std::move(classes)), pseudo_class_names(std::move(pseudo_classes)),
-	structural_selectors(std::move(structural_selectors)), combinator(combinator)
+StyleSheetNode::StyleSheetNode(StyleSheetNode* parent, CompoundSelector&& selector) : parent(parent), selector(std::move(selector))
 {
 	CalculateAndSetSpecificity();
 }
 
-StyleSheetNode* StyleSheetNode::GetOrCreateChildNode(const StyleSheetNode& other)
+StyleSheetNode* StyleSheetNode::GetOrCreateChildNode(const CompoundSelector& other)
 {
-	// See if we match the target child
+	// See if we match an existing child
 	for (const auto& child : children)
 	{
-		if (child->EqualRequirements(other.tag, other.id, other.class_names, other.pseudo_class_names, other.structural_selectors, other.combinator))
+		if (child->selector == other)
 			return child.get();
 	}
 
 	// We don't, so create a new child
-	auto child = MakeUnique<StyleSheetNode>(this, other.tag, other.id, other.class_names, other.pseudo_class_names, other.structural_selectors,
-		other.combinator);
+	auto child = MakeUnique<StyleSheetNode>(this, other);
 	StyleSheetNode* result = child.get();
 
 	children.push_back(std::move(child));
@@ -82,19 +75,17 @@ StyleSheetNode* StyleSheetNode::GetOrCreateChildNode(const StyleSheetNode& other
 	return result;
 }
 
-StyleSheetNode* StyleSheetNode::GetOrCreateChildNode(String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes,
-	StructuralSelectorList&& structural_pseudo_classes, SelectorCombinator combinator)
+StyleSheetNode* StyleSheetNode::GetOrCreateChildNode(CompoundSelector&& other)
 {
 	// See if we match an existing child
 	for (const auto& child : children)
 	{
-		if (child->EqualRequirements(tag, id, classes, pseudo_classes, structural_pseudo_classes, combinator))
+		if (child->selector == other)
 			return child.get();
 	}
 
 	// We don't, so create a new child
-	auto child = MakeUnique<StyleSheetNode>(this, std::move(tag), std::move(id), std::move(classes), std::move(pseudo_classes),
-		std::move(structural_pseudo_classes), combinator);
+	auto child = MakeUnique<StyleSheetNode>(this, std::move(other));
 	StyleSheetNode* result = child.get();
 
 	children.push_back(std::move(child));
@@ -112,7 +103,7 @@ void StyleSheetNode::MergeHierarchy(StyleSheetNode* node, int specificity_offset
 
 	for (const auto& other_child : node->children)
 	{
-		StyleSheetNode* local_node = GetOrCreateChildNode(*other_child);
+		StyleSheetNode* local_node = GetOrCreateChildNode(other_child->selector);
 		local_node->MergeHierarchy(other_child.get(), specificity_offset);
 	}
 }
@@ -121,7 +112,7 @@ UniquePtr<StyleSheetNode> StyleSheetNode::DeepCopy(StyleSheetNode* in_parent) co
 {
 	RMLUI_ZoneScoped;
 
-	auto node = MakeUnique<StyleSheetNode>(in_parent, tag, id, class_names, pseudo_class_names, structural_selectors, combinator);
+	auto node = MakeUnique<StyleSheetNode>(in_parent, selector);
 
 	node->properties = properties;
 	node->children.resize(children.size());
@@ -149,19 +140,19 @@ void StyleSheetNode::BuildIndex(StyleSheetIndex& styled_node_index) const
 
 		// Add this node to the appropriate index for looking up applicable nodes later. Prioritize the most unique requirement first and the most
 		// general requirement last. This way we are able to rule out as many nodes as possible as quickly as possible.
-		if (!id.empty())
+		if (!selector.id.empty())
 		{
-			IndexInsertNode(styled_node_index.ids, id, this);
+			IndexInsertNode(styled_node_index.ids, selector.id, this);
 		}
-		else if (!class_names.empty())
+		else if (!selector.class_names.empty())
 		{
 			// @performance Right now we just use the first class for simplicity. Later we may want to devise a better strategy to try to add the
 			// class with the most unique name. For example by adding the class from this node's list that has the fewest existing matches.
-			IndexInsertNode(styled_node_index.classes, class_names.front(), this);
+			IndexInsertNode(styled_node_index.classes, selector.class_names.front(), this);
 		}
-		else if (!tag.empty())
+		else if (!selector.tag.empty())
 		{
-			IndexInsertNode(styled_node_index.tags, tag, this);
+			IndexInsertNode(styled_node_index.tags, selector.tag, this);
 		}
 		else
 		{
@@ -173,43 +164,6 @@ void StyleSheetNode::BuildIndex(StyleSheetIndex& styled_node_index) const
 		child->BuildIndex(styled_node_index);
 }
 
-bool StyleSheetNode::SetStructurallyVolatileRecursive(bool ancestor_is_structural_pseudo_class)
-{
-	// If any ancestor or descendant is a structural pseudo class, then we are structurally volatile.
-	bool self_is_structural_pseudo_class = (!structural_selectors.empty());
-
-	// Check our children for structural pseudo-classes.
-	bool descendant_is_structural_pseudo_class = false;
-	for (auto& child : children)
-	{
-		if (child->SetStructurallyVolatileRecursive(self_is_structural_pseudo_class || ancestor_is_structural_pseudo_class))
-			descendant_is_structural_pseudo_class = true;
-	}
-
-	is_structurally_volatile = (self_is_structural_pseudo_class || ancestor_is_structural_pseudo_class || descendant_is_structural_pseudo_class);
-
-	return (self_is_structural_pseudo_class || descendant_is_structural_pseudo_class);
-}
-
-bool StyleSheetNode::EqualRequirements(const String& _tag, const String& _id, const StringList& _class_names, const StringList& _pseudo_class_names,
-	const StructuralSelectorList& _structural_selectors, SelectorCombinator _combinator) const
-{
-	if (tag != _tag)
-		return false;
-	if (id != _id)
-		return false;
-	if (class_names != _class_names)
-		return false;
-	if (pseudo_class_names != _pseudo_class_names)
-		return false;
-	if (structural_selectors != _structural_selectors)
-		return false;
-	if (combinator != _combinator)
-		return false;
-
-	return true;
-}
-
 // Returns the specificity of this node.
 int StyleSheetNode::GetSpecificity() const
 {
@@ -231,41 +185,36 @@ const PropertyDictionary& StyleSheetNode::GetProperties() const
 
 bool StyleSheetNode::Match(const Element* element) const
 {
-	if (!tag.empty() && tag != element->GetTagName())
-		return false;
-
-	if (!id.empty() && id != element->GetId())
-		return false;
-
-	if (!MatchClassPseudoClass(element))
+	if (!selector.tag.empty() && selector.tag != element->GetTagName())
 		return false;
 
-	if (!MatchStructuralSelector(element))
+	if (!selector.id.empty() && selector.id != element->GetId())
 		return false;
 
-	return true;
-}
-
-inline bool StyleSheetNode::MatchClassPseudoClass(const Element* element) const
-{
-	for (auto& name : class_names)
+	for (auto& name : selector.class_names)
 	{
 		if (!element->IsClassSet(name))
 			return false;
 	}
 
-	for (auto& name : pseudo_class_names)
+	for (auto& name : selector.pseudo_class_names)
 	{
 		if (!element->IsPseudoClassSet(name))
 			return false;
 	}
 
+	if (!selector.attributes.empty() && !MatchAttributes(element))
+		return false;
+
+	if (!selector.structural_selectors.empty() && !MatchStructuralSelector(element))
+		return false;
+
 	return true;
 }
 
-inline bool StyleSheetNode::MatchStructuralSelector(const Element* element) const
+bool StyleSheetNode::MatchStructuralSelector(const Element* element) const
 {
-	for (auto& node_selector : structural_selectors)
+	for (auto& node_selector : selector.structural_selectors)
 	{
 		if (!IsSelectorApplicable(element, node_selector))
 			return false;
@@ -274,83 +223,164 @@ inline bool StyleSheetNode::MatchStructuralSelector(const Element* element) cons
 	return true;
 }
 
-bool StyleSheetNode::IsApplicable(const Element* const in_element) const
+bool StyleSheetNode::MatchAttributes(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 .
-	for (const String& name : pseudo_class_names)
+	for (const AttributeSelector& attribute : selector.attributes)
 	{
-		if (!in_element->IsPseudoClassSet(name))
+		const Variant* variant = element->GetAttribute(attribute.name);
+		if (!variant)
 			return false;
-	}
-
-	if (!tag.empty() && tag != in_element->GetTagName())
-		return false;
+		if (attribute.type == AttributeSelectorType::Always)
+			continue;
 
-	for (const String& name : class_names)
-	{
-		if (!in_element->IsClassSet(name))
-			return false;
-	}
+		String buffer;
+		const String* element_value_ptr = &buffer;
+		if (variant->GetType() == Variant::STRING)
+			element_value_ptr = &variant->GetReference<String>();
+		else
+			variant->GetInto(buffer);
 
-	if (!id.empty() && id != in_element->GetId())
-		return false;
+		const String& element_value = *element_value_ptr;
+		const String& css_value = attribute.value;
 
-	const Element* element = in_element;
+		auto BeginsWith = [](const String& target, const String& prefix) {
+			return prefix.size() <= target.size() && std::equal(prefix.begin(), prefix.end(), target.begin());
+		};
+		auto EndsWith = [](const String& target, const String& suffix) {
+			return suffix.size() <= target.size() && std::equal(suffix.rbegin(), suffix.rend(), target.rbegin());
+		};
 
-	// 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)
+		switch (attribute.type)
 		{
-		case SelectorCombinator::None:
-		case SelectorCombinator::Child:
+		case AttributeSelectorType::Always: break;
+		case AttributeSelectorType::Equal:
+			if (element_value != css_value)
+				return false;
+			break;
+		case AttributeSelectorType::InList:
 		{
-			// 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())
+			bool found = false;
+			for (size_t index = element_value.find(css_value); index != String::npos; index = element_value.find(css_value, index + 1))
 			{
-				if (node->Match(element))
+				const size_t index_right = index + css_value.size();
+				const bool whitespace_left = (index == 0 || element_value[index - 1] == ' ');
+				const bool whitespace_right = (index_right == element_value.size() || element_value[index_right] == ' ');
+
+				if (whitespace_left && whitespace_right)
+				{
+					found = true;
 					break;
-				// If the node has a child combinator we must match this first ancestor.
-				else if (node->combinator == SelectorCombinator::Child)
-					return false;
+				}
 			}
+			if (!found)
+				return false;
 		}
 		break;
-		case SelectorCombinator::NextSibling:
-		case SelectorCombinator::SubsequentSibling:
+		case AttributeSelectorType::BeginsWithThenHyphen:
+			if (!BeginsWith(element_value, css_value) || element_value[css_value.size()] != '-')
+				return false;
+			break;
+		case AttributeSelectorType::BeginsWith:
+			if (!BeginsWith(element_value, css_value))
+				return false;
+			break;
+		case AttributeSelectorType::EndsWith:
+			if (!EndsWith(element_value, css_value))
+				return false;
+			break;
+		case AttributeSelectorType::Contains:
+			if (element_value.find(css_value) == String::npos)
+				return false;
+			break;
+		}
+	}
+	return true;
+}
+
+bool StyleSheetNode::TraverseMatch(const Element* element) const
+{
+	RMLUI_ASSERT(parent);
+	if (!parent->parent)
+		return true;
+
+	switch (selector.combinator)
+	{
+	case SelectorCombinator::Descendant:
+	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())
 		{
-			// 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;
-			}
+			if (parent->Match(element) && parent->TraverseMatch(element))
+				return true;
+			// If the node has a child combinator we must match this first ancestor.
+			else if (selector.combinator == SelectorCombinator::Child)
+				return false;
 		}
-		break;
+	}
+	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 (parent->Match(element) && parent->TraverseMatch(element))
+				return true;
+			// If the node has a next-sibling combinator we must match this first sibling.
+			else if (selector.combinator == SelectorCombinator::NextSibling)
+				return false;
 		}
+	}
+	break;
+	}
+
+	// We have run out of element ancestors before we matched every node. Bail out.
+	return false;
+}
 
-		// We have run out of element ancestors before we matched every node. Bail out.
-		if (!element)
+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.
+	for (const String& name : selector.pseudo_class_names)
+	{
+		if (!element->IsPseudoClassSet(name))
 			return false;
 	}
 
-	// Finally, check the structural selector requirements last as they can be quite slow.
-	if (!MatchStructuralSelector(in_element))
+	if (!selector.tag.empty() && selector.tag != element->GetTagName())
 		return false;
 
-	return true;
-}
+	for (const String& name : selector.class_names)
+	{
+		if (!element->IsClassSet(name))
+			return false;
+	}
 
-bool StyleSheetNode::IsStructurallyVolatile() const
-{
-	return is_structurally_volatile;
+	if (!selector.id.empty() && selector.id != element->GetId())
+		return false;
+
+	if (!selector.attributes.empty() && !MatchAttributes(element))
+		return false;
+
+	// Check the structural selector requirements last as they can be quite slow.
+	if (!selector.structural_selectors.empty() && !MatchStructuralSelector(element))
+		return false;
+
+	// Walk up through all our parent nodes, each one of them must be matched by some ancestor or sibling element.
+	if (parent && !TraverseMatch(element))
+		return false;
+
+	return true;
 }
 
 void StyleSheetNode::CalculateAndSetSpecificity()
@@ -358,17 +388,18 @@ void StyleSheetNode::CalculateAndSetSpecificity()
 	// First calculate the specificity of this node alone.
 	specificity = 0;
 
-	if (!tag.empty())
+	if (!selector.tag.empty())
 		specificity += SelectorSpecificity::Tag;
 
-	if (!id.empty())
+	if (!selector.id.empty())
 		specificity += SelectorSpecificity::ID;
 
-	specificity += SelectorSpecificity::Class * (int)class_names.size();
-	specificity += SelectorSpecificity::PseudoClass * (int)pseudo_class_names.size();
-	
-	for (const StructuralSelector& selector : structural_selectors)
-		specificity += selector.specificity; 
+	specificity += SelectorSpecificity::Class * (int)selector.class_names.size();
+	specificity += SelectorSpecificity::Attribute * (int)selector.attributes.size();
+	specificity += SelectorSpecificity::PseudoClass * (int)selector.pseudo_class_names.size();
+
+	for (const StructuralSelector& selector : selector.structural_selectors)
+		specificity += selector.specificity;
 
 	// Then add our parent's specificity onto ours.
 	if (parent)

+ 10 - 28
Source/Core/StyleSheetNode.h

@@ -37,7 +37,6 @@ namespace Rml {
 
 struct StyleSheetIndex;
 class StyleSheetNode;
-using StructuralSelectorList = Vector<StructuralSelector>;
 using StyleSheetNodeList = Vector<UniquePtr<StyleSheetNode>>;
 
 /**
@@ -49,23 +48,18 @@ using StyleSheetNodeList = Vector<UniquePtr<StyleSheetNode>>;
 class StyleSheetNode {
 public:
 	StyleSheetNode();
-	StyleSheetNode(StyleSheetNode* parent, const String& tag, const String& id, const StringList& classes, const StringList& pseudo_classes,
-		const StructuralSelectorList& structural_selectors, SelectorCombinator combinator);
-	StyleSheetNode(StyleSheetNode* parent, String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes,
-		StructuralSelectorList&& structural_selectors, SelectorCombinator combinator);
+	StyleSheetNode(StyleSheetNode* parent, const CompoundSelector& selector);
+	StyleSheetNode(StyleSheetNode* parent, CompoundSelector&& selector);
 
-	/// Retrieves a child node with the given requirements if they match an existing node, or else creates a new one.
-	StyleSheetNode* GetOrCreateChildNode(String&& tag, String&& id, StringList&& classes, StringList&& pseudo_classes,
-		StructuralSelectorList&& structural_selectors, SelectorCombinator combinator);
 	/// Retrieves or creates a child node with requirements equivalent to the 'other' node.
-	StyleSheetNode* GetOrCreateChildNode(const StyleSheetNode& other);
+	StyleSheetNode* GetOrCreateChildNode(const CompoundSelector& other);
+	/// Retrieves a child node with the given requirements if they match an existing node, or else creates a new one.
+	StyleSheetNode* GetOrCreateChildNode(CompoundSelector&& other);
 
 	/// Merges an entire tree hierarchy into our hierarchy.
 	void MergeHierarchy(StyleSheetNode* node, int specificity_offset = 0);
 	/// Copy this node including all descendent nodes.
 	UniquePtr<StyleSheetNode> DeepCopy(StyleSheetNode* parent = nullptr) const;
-	/// Recursively set structural volatility.
-	bool SetStructurallyVolatileRecursive(bool ancestor_is_structurally_volatile);
 	/// Builds up a style sheet's index recursively.
 	void BuildIndex(StyleSheetIndex& styled_node_index) const;
 
@@ -84,35 +78,23 @@ public:
 
 	/// Returns the specificity of this node.
 	int GetSpecificity() const;
-	/// Returns true if this node employs a structural selector, and therefore generates element definitions that are sensitive to sibling changes.
-	/// @warning Result is only valid if structural volatility is set since any changes to the node tree.
-	bool IsStructurallyVolatile() const;
 
 private:
-	// Returns true if the requirements of this node equals the given arguments.
-	bool EqualRequirements(const String& tag, const String& id, const StringList& classes, const StringList& pseudo_classes,
-		const StructuralSelectorList& structural_pseudo_classes, SelectorCombinator combinator) const;
-
 	void CalculateAndSetSpecificity();
 
 	// Match an element to the local node requirements.
 	inline bool Match(const Element* element) const;
-	inline bool MatchClassPseudoClass(const Element* element) const;
 	inline bool MatchStructuralSelector(const Element* element) const;
+	inline bool MatchAttributes(const Element* element) const;
+
+	// Recursively traverse the nodes up towards the root to match the element and its hierarchy.
+	bool TraverseMatch(const Element* element) const;
 
 	// The parent of this node; is nullptr for the root node.
 	StyleSheetNode* parent = nullptr;
 
 	// Node requirements
-	String tag;
-	String id;
-	StringList class_names;
-	StringList pseudo_class_names;
-	StructuralSelectorList structural_selectors; // Represents structural pseudo classes
-	SelectorCombinator combinator = SelectorCombinator::None;
-
-	// True if any ancestor, descendent, or self is a structural pseudo class.
-	bool is_structurally_volatile = true;
+	CompoundSelector selector;
 
 	// A measure of specificity of this node; the attribute in a node with a higher value will override those of a node with a lower value.
 	int specificity = 0;

+ 109 - 79
Source/Core/StyleSheetParser.cpp

@@ -565,7 +565,11 @@ bool StyleSheetParser::Parse(MediaBlockList& style_sheets, Stream* _stream, int
 					{
 						auto source = MakeShared<PropertySource>(stream_file_name, rule_line_number, rule_name_list[i]);
 						properties.SetSourceOfAllProperties(source);
-						ImportProperties(current_block.stylesheet->root.get(), rule_name_list[i], properties, rule_count);
+						if (!ImportProperties(current_block.stylesheet->root.get(), rule_name_list[i], properties, rule_count))
+						{
+							Log::Message(Log::LT_WARNING, "Invalid selector '%s' encountered while parsing stylesheet at %s:%d.",
+								rule_name_list[i].c_str(), stream_file_name.c_str(), line_number);
+						}
 					}
 
 					rule_count++;
@@ -762,7 +766,9 @@ StyleSheetNodeListRaw StyleSheetParser::ConstructNodes(StyleSheetNode& root_node
 	{
 		StyleSheetNode* leaf_node = ImportProperties(&root_node, selector, empty_properties, 0);
 
-		if (leaf_node != &root_node)
+		if (!leaf_node)
+			Log::Message(Log::LT_WARNING, "Invalid selector '%s' encountered.", selector.c_str());
+		else if (leaf_node != &root_node)
 			leaf_nodes.push_back(leaf_node);
 	}
 
@@ -870,116 +876,140 @@ bool StyleSheetParser::ReadProperties(AbstractPropertyParser& property_parser)
 	return true;
 }
 
-StyleSheetNode* StyleSheetParser::ImportProperties(StyleSheetNode* node, String rule_name, const PropertyDictionary& properties, int rule_specificity)
+StyleSheetNode* StyleSheetParser::ImportProperties(StyleSheetNode* node, const String& rule, const PropertyDictionary& properties,
+	int rule_specificity)
 {
 	StyleSheetNode* leaf_node = node;
 
-	StringList nodes;
-
-	// Combinator rules can be formatted several ways by users, here we ensure consistent formatting before we can continue with the parsing below.
-	// E.g. converts combinations such as "A > B", "A >B", "A>B"  to  "A> B".
-	for (const char combinator : {'>', '+', '~'})
+	// Create each node going down the tree.
+	for (size_t index = 0; index < rule.size();)
 	{
-		// Find all combinators of the given type.
-		size_t i_child = rule_name.find(combinator);
-		while (i_child != String::npos)
+		CompoundSelector selector;
+
+		// Determine the combinator connecting the previous node if any.
+		for (; index > 0 && index < rule.size(); index++)
 		{
-			// So we found one! Next, we want to format the rule such that e.g. the '>' is located at the
-			// end of the left-hand-side node, and that there is a space to the right-hand-side. This ensures that
-			// the selector is applied to the "parent", and that parent and child are expanded properly below.
-			size_t i_begin = i_child;
-			while (i_begin > 0 && rule_name[i_begin - 1] == ' ')
-				i_begin--;
-
-			const size_t i_end = i_child + 1;
-			const char replacement_str[] = {combinator, ' ', '\0'};
-			rule_name.replace(i_begin, i_end - i_begin, (const char*)replacement_str);
-			i_child = rule_name.find(combinator, i_begin + 1);
+			bool reached_end_of_combinators = false;
+			switch (rule[index])
+			{
+			case ' ': break;
+			case '>': selector.combinator = SelectorCombinator::Child; break;
+			case '+': selector.combinator = SelectorCombinator::NextSibling; break;
+			case '~': selector.combinator = SelectorCombinator::SubsequentSibling; break;
+			default: reached_end_of_combinators = true; break;
+			}
+			if (reached_end_of_combinators)
+				break;
 		}
-	}
-
-	// Expand each individual node separated by spaces. Don't expand inside parenthesis because of structural selectors.
-	StringUtilities::ExpandString(nodes, rule_name, ' ', '(', ')', true);
-
-	// Create each node going down the tree
-	for (size_t i = 0; i < nodes.size(); i++)
-	{
-		const String& name = nodes[i];
-
-		String tag;
-		String id;
-		StringList classes;
-		StringList pseudo_classes;
-		StructuralSelectorList structural_pseudo_classes;
-		SelectorCombinator combinator = SelectorCombinator::None;
 
-		size_t index = 0;
-		while (index < name.size())
+		// Determine the node's requirements.
+		while (index < rule.size())
 		{
 			size_t start_index = index;
 			size_t end_index = index + 1;
-			int parenthesis_count = 0;
 
-			// Read until we hit the next identifier.
-			for (; end_index < name.size(); end_index++)
+			if (rule[start_index] == '*')
+				start_index += 1;
+
+			if (rule[start_index] == '[')
 			{
-				static const String identifiers = "#.:>+~";
-				if (parenthesis_count == 0 && identifiers.find(name[end_index]) != String::npos)
-					break;
+				end_index = rule.find(']', start_index + 1);
+				if (end_index == String::npos)
+					return nullptr;
+				end_index += 1;
+			}
+			else
+			{
+				int parenthesis_count = 0;
 
-				if (name[end_index] == '(')
-					parenthesis_count += 1;
-				else if (name[end_index] == ')')
-					parenthesis_count -= 1;
+				// Read until we hit the next identifier. Don't match inside parenthesis in case of structural selectors.
+				for (; end_index < rule.size(); end_index++)
+				{
+					static const String identifiers = "#.:[ >+~";
+					if (parenthesis_count == 0 && identifiers.find(rule[end_index]) != String::npos)
+						break;
+
+					if (rule[end_index] == '(')
+						parenthesis_count += 1;
+					else if (rule[end_index] == ')')
+						parenthesis_count -= 1;
+				}
 			}
 
-			String identifier = name.substr(start_index, end_index - start_index);
-			if (!identifier.empty())
+			if (end_index > start_index)
 			{
-				switch (identifier[0])
+				const char* p_begin = rule.data() + start_index;
+				const char* p_end = rule.data() + end_index;
+
+				switch (rule[start_index])
 				{
-				case '#':
-					id = identifier.substr(1);
-					break;
-				case '.':
-					classes.push_back(identifier.substr(1));
-					break;
+				case '#': selector.id = String(p_begin + 1, p_end); break;
+				case '.': selector.class_names.push_back(String(p_begin + 1, p_end)); break;
 				case ':':
 				{
-					String pseudo_class_name = identifier.substr(1);
+					String pseudo_class_name = String(p_begin + 1, p_end);
 					StructuralSelector node_selector = StyleSheetFactory::GetSelector(pseudo_class_name);
 					if (node_selector.type != StructuralSelectorType::Invalid)
-						structural_pseudo_classes.push_back(node_selector);
+						selector.structural_selectors.push_back(node_selector);
 					else
-						pseudo_classes.push_back(pseudo_class_name);
+						selector.pseudo_class_names.push_back(std::move(pseudo_class_name));
 				}
 				break;
-				case '>':
-					combinator = SelectorCombinator::Child;
-					break;
-				case '+':
-					combinator = SelectorCombinator::NextSibling;
-					break;
-				case '~':
-					combinator = SelectorCombinator::SubsequentSibling;
-					break;
+				case '[':
+				{
+					const size_t i_attr_begin = start_index + 1;
+					const size_t i_attr_end = end_index - 1;
+					if (i_attr_end <= i_attr_begin)
+						return nullptr;
+
+					AttributeSelector attribute;
+
+					static const String attribute_operators = "=~|^$*]";
+					size_t i_cursor = Math::Min(rule.find_first_of(attribute_operators, i_attr_begin), i_attr_end);
+					attribute.name = rule.substr(i_attr_begin, i_cursor - i_attr_begin);
+
+					if (i_cursor < i_attr_end)
+					{
+						const char c = rule[i_cursor];
+						attribute.type = AttributeSelectorType(c);
 
-				default:
-					if (identifier != "*")
-						tag = identifier;
+						// Move cursor past operator. Non-'=' symbols are always followed by '=' so move two characters.
+						i_cursor += (c == '=' ? 1 : 2);
+
+						size_t i_value_end = i_attr_end;
+						if (i_cursor < i_attr_end && (rule[i_cursor] == '"' || rule[i_cursor] == '\''))
+						{
+							i_cursor += 1;
+							i_value_end -= 1;
+						}
+
+						if (i_cursor < i_value_end)
+							attribute.value = rule.substr(i_cursor, i_value_end - i_cursor);
+					}
+
+					selector.attributes.push_back(std::move(attribute));
+				}
+				break;
+				default: selector.tag = String(p_begin, p_end); break;
 				}
 			}
 
 			index = end_index;
+
+			// If we reached a combinator then we submit the current node and start fresh with a new node.
+			static const String combinators(" >+~");
+			if (combinators.find(rule[index]) != String::npos)
+				break;
 		}
 
 		// Sort the classes and pseudo-classes so they are consistent across equivalent declarations that shuffle the order around.
-		std::sort(classes.begin(), classes.end());
-		std::sort(pseudo_classes.begin(), pseudo_classes.end());
-		std::sort(structural_pseudo_classes.begin(), structural_pseudo_classes.end());
+		std::sort(selector.class_names.begin(), selector.class_names.end());
+		std::sort(selector.attributes.begin(), selector.attributes.end());
+		std::sort(selector.pseudo_class_names.begin(), selector.pseudo_class_names.end());
+		std::sort(selector.structural_selectors.begin(), selector.structural_selectors.end());
 
-		// Get the named child node.
-		leaf_node = leaf_node->GetOrCreateChildNode(std::move(tag), std::move(id), std::move(classes), std::move(pseudo_classes), std::move(structural_pseudo_classes), combinator);
+		// Add the new child node, or retrieve the existing child if we have an exact match.
+		leaf_node = leaf_node->GetOrCreateChildNode(std::move(selector));
 	}
 
 	// Merge the new properties with those already on the leaf node.

+ 3 - 3
Source/Core/StyleSheetParser.h

@@ -96,11 +96,11 @@ private:
 
 	// Import properties into the stylesheet node
 	// @param node Node to import into
-	// @param names The names of the nodes
+	// @param rule The rule name to parse
 	// @param properties The dictionary of properties
 	// @param rule_specificity The specifity of the rule
-	// @return The leaf node of the rule
-	static StyleSheetNode* ImportProperties(StyleSheetNode* node, String rule_name, const PropertyDictionary& properties, int rule_specificity);
+	// @return The leaf node of the rule, or nullptr on parse failure.
+	static StyleSheetNode* ImportProperties(StyleSheetNode* node, const String& rule, const PropertyDictionary& properties, int rule_specificity);
 
 	// Attempts to parse a @keyframes block
 	bool ParseKeyframeBlock(KeyframesMap & keyframes_map, const String & identifier, const String & rules, const PropertyDictionary & properties);

+ 42 - 0
Source/Core/StyleSheetSelector.cpp

@@ -29,6 +29,7 @@
 #include "StyleSheetSelector.h"
 #include "../../Include/RmlUi/Core/Element.h"
 #include "StyleSheetNode.h"
+#include <tuple>
 
 namespace Rml {
 
@@ -48,6 +49,47 @@ static bool IsNth(int a, int b, int count)
 	return (x >= 0 && x * a + b == count);
 }
 
+bool operator==(const AttributeSelector& a, const AttributeSelector& b)
+{
+	return a.type == b.type && a.name == b.name && a.value == b.value;
+}
+bool operator<(const AttributeSelector& a, const AttributeSelector& b)
+{
+	return std::tie(a.type, a.name, a.value) < std::tie(b.type, b.name, b.value);
+}
+
+bool operator==(const StructuralSelector& a, const StructuralSelector& b)
+{
+	// Currently sub-selectors (selector_tree) are only superficially compared. This mainly has the consequence that selectors with a sub-selector
+	// which are instantiated separately will never compare equal, even if they have the exact same sub-selector expression. This further results in
+	// such selectors not being de-duplicated. This should not lead to any functional differences but leads to potentially missed memory/performance
+	// optimizations. E.g. 'div a, div b' will combine the two div nodes, while ':not(div) a, :not(div) b' will not combine the two not-div nodes.
+	return a.type == b.type && a.a == b.a && a.b == b.b && a.selector_tree == b.selector_tree;
+}
+bool operator<(const StructuralSelector& a, const StructuralSelector& b)
+{
+	return std::tie(a.type, a.a, a.b, a.selector_tree) < std::tie(b.type, b.a, b.b, b.selector_tree);
+}
+
+bool operator==(const CompoundSelector& a, const CompoundSelector& b)
+{
+	if (a.tag != b.tag)
+		return false;
+	if (a.id != b.id)
+		return false;
+	if (a.class_names != b.class_names)
+		return false;
+	if (a.pseudo_class_names != b.pseudo_class_names)
+		return false;
+	if (a.attributes != b.attributes)
+		return false;
+	if (a.structural_selectors != b.structural_selectors)
+		return false;
+	if (a.combinator != b.combinator)
+		return false;
+	return true;
+}
+
 bool IsSelectorApplicable(const Element* element, const StructuralSelector& selector)
 {
 	RMLUI_ASSERT(element);

+ 66 - 27
Source/Core/StyleSheetSelector.h

@@ -30,24 +30,71 @@
 #define RMLUI_CORE_STYLESHEETSELECTOR_H
 
 #include "../../Include/RmlUi/Core/Types.h"
-#include <tuple>
 
 namespace Rml {
 
 class Element;
 class StyleSheetNode;
-struct SelectorTree;
 
+/**
+    Constants used to determine the specificity of a selector.
+ */
 namespace SelectorSpecificity {
 	enum {
-		// Constants used to determine the specificity of a selector.
 		Tag = 10'000,
 		Class = 100'000,
+		Attribute = Class,
 		PseudoClass = Class,
 		ID = 1'000'000,
 	};
 }
 
+/**
+    Combinator determines how two connected compound selectors are matched against the element hierarchy.
+ */
+enum class SelectorCombinator {
+	Descendant,        // The 'E F' (whitespace) combinator: Matches if F is a descendant of E.
+	Child,             // The 'E > F' combinator: Matches if F is a child of E.
+	NextSibling,       // The 'E + F' combinator: Matches if F is immediately preceded by E.
+	SubsequentSibling, // The 'E ~ F' combinator: Matches if F is preceded by E.
+};
+
+/**
+    Attribute selector.
+
+    Such as [unit], [unit=meter], [href^=http]
+ */
+enum class AttributeSelectorType {
+	Always,
+	Equal = '=',
+	InList = '~',
+	BeginsWithThenHyphen = '|',
+	BeginsWith = '^',
+	EndsWith = '$',
+	Contains = '*',
+};
+struct AttributeSelector {
+	AttributeSelectorType type = AttributeSelectorType::Always;
+	String name;
+	String value;
+};
+bool operator==(const AttributeSelector& a, const AttributeSelector& b);
+bool operator<(const AttributeSelector& a, const AttributeSelector& b);
+using AttributeSelectorList = Vector<AttributeSelector>;
+
+/**
+    A tree of unstyled style sheet nodes.
+ */
+struct SelectorTree {
+	UniquePtr<StyleSheetNode> root;
+	Vector<StyleSheetNode*> leafs; // Owned by root.
+};
+
+/**
+    Tree-structural selector.
+
+    Such as :nth-child(2n+1), :empty(), :not(div)
+ */
 enum class StructuralSelectorType {
 	Invalid,
 	Nth_Child,
@@ -63,7 +110,6 @@ enum class StructuralSelectorType {
 	Empty,
 	Not
 };
-
 struct StructuralSelector {
 	StructuralSelector(StructuralSelectorType type, int a, int b) : type(type), a(a), b(b) {}
 	StructuralSelector(StructuralSelectorType type, SharedPtr<const SelectorTree> tree, int specificity) :
@@ -82,32 +128,25 @@ struct StructuralSelector {
 	// For selectors that contain internal selectors such as :not().
 	SharedPtr<const SelectorTree> selector_tree;
 };
+bool operator==(const StructuralSelector& a, const StructuralSelector& b);
+bool operator<(const StructuralSelector& a, const StructuralSelector& b);
+using StructuralSelectorList = Vector<StructuralSelector>;
 
-inline bool operator==(const StructuralSelector& a, const StructuralSelector& b)
-{
-	// Currently sub-selectors (selector_tree) are only superficially compared. This mainly has the consequence that selectors with a sub-selector
-	// which are instantiated separately will never compare equal, even if they have the exact same sub-selector expression. This further results in
-	// such selectors not being de-duplicated. This should not lead to any functional differences but leads to potentially missed memory/performance
-	// optimizations. E.g. 'div a, div b' will combine the two div nodes, while ':not(div) a, :not(div) b' will not combine the two not-div nodes.
-	return a.type == b.type && a.a == b.a && a.b == b.b && a.selector_tree == b.selector_tree;
-}
-inline bool operator<(const StructuralSelector& a, const StructuralSelector& b)
-{
-	return std::tie(a.type, a.a, a.b, a.selector_tree) < std::tie(b.type, b.a, b.b, b.selector_tree);
-}
-
-// A tree of unstyled style sheet nodes.
-struct SelectorTree {
-	UniquePtr<StyleSheetNode> root;
-	Vector<StyleSheetNode*> leafs; // Owned by root.
-};
+/**
+    Compound selector contains all the basic selectors for a single node.
 
-enum class SelectorCombinator : byte {
-	None,
-	Child,             // The 'E > F' combinator: Matches if F is a child of E.
-	NextSibling,       // The 'E + F' combinator: Matches if F is immediately preceded by E.
-	SubsequentSibling, // The 'E ~ F' combinator: Matches if F is preceded by E.
+    Such as div#foo.bar:nth-child(2)
+ */
+struct CompoundSelector {
+	String tag;
+	String id;
+	StringList class_names;
+	StringList pseudo_class_names;
+	AttributeSelectorList attributes;
+	StructuralSelectorList structural_selectors;
+	SelectorCombinator combinator = SelectorCombinator::Descendant; // Determines how to match with our parent node.
 };
+bool operator==(const CompoundSelector& a, const CompoundSelector& b);
 
 /// Returns true if the the node the given selector is discriminating for is applicable to a given element.
 /// @param element[in] The element to determine node applicability for.

+ 26 - 48
Tests/Source/Benchmarks/Element.cpp

@@ -31,7 +31,6 @@
 #include <RmlUi/Core/Element.h>
 #include <RmlUi/Core/ElementDocument.h>
 #include <RmlUi/Core/Types.h>
-
 #include <doctest.h>
 #include <nanobench.h>
 
@@ -67,7 +66,6 @@ static const String document_rml = R"(
 </rml>
 )";
 
-
 static int GetNumDescendentElements(Element* element)
 {
 	const int num_children = element->GetNumChildren(true);
@@ -79,7 +77,6 @@ static int GetNumDescendentElements(Element* element)
 	return result;
 }
 
-
 static String GenerateRml(const int num_rows)
 {
 	static nanobench::Rng rng;
@@ -108,18 +105,13 @@ static String GenerateRml(const int num_rows)
 					</div>
 				</div>
 			</div>)",
-			index,
-			route,
-			max,
-			value
-		);
+			index, route, max, value);
 		rml += rml_row;
 	}
 
 	return rml;
 }
 
-
 TEST_CASE("element.creation_and_destruction")
 {
 	Context* context = TestsShell::GetContext();
@@ -148,17 +140,26 @@ TEST_CASE("element.creation_and_destruction")
 	bench.timeUnit(std::chrono::microseconds(1), "us");
 	bench.relative(true);
 
-	bench.run("Update (unmodified)", [&] {
+	bench.run("Update (unmodified)", [&] { context->Update(); });
+
+	bool hover_toggle = true;
+	auto child = el->GetChild(num_rows / 2);
+
+	bench.run("Update (hover child)", [&] {
+		static nanobench::Rng rng;
+		child->SetPseudoClass(":hover", hover_toggle);
+		hover_toggle = !hover_toggle;
 		context->Update();
 	});
-
-	bench.run("Render", [&] {
-		context->Render();
+	bench.run("Update (hover)", [&] {
+		el->SetPseudoClass(":hover", hover_toggle);
+		hover_toggle = !hover_toggle;
+		context->Update();
 	});
 
-	bench.run("SetInnerRML", [&] {
-		el->SetInnerRML(rml);
-	});
+	bench.run("Render", [&] { context->Render(); });
+
+	bench.run("SetInnerRML", [&] { el->SetInnerRML(rml); });
 
 	bench.run("SetInnerRML + Update", [&] {
 		el->SetInnerRML(rml);
@@ -174,7 +175,6 @@ TEST_CASE("element.creation_and_destruction")
 	document->Close();
 }
 
-
 TEST_CASE("element.asymptotic_complexity")
 {
 	Context* context = TestsShell::GetContext();
@@ -187,46 +187,26 @@ TEST_CASE("element.asymptotic_complexity")
 	Element* el = document->GetElementById("performance");
 	REQUIRE(el);
 
-
 	struct BenchDef {
 		const char* title;
 		Function<void(const String& rml)> run;
 	};
 
 	Vector<BenchDef> bench_list = {
-		{
-			"SetInnerRML",
+		{"SetInnerRML", [&](const String& rml) { el->SetInnerRML(rml); }},
+		{"Update (unmodified)", [&](const String& /*rml*/) { context->Update(); }},
+		{"Render", [&](const String& /*rml*/) { context->Render(); }},
+		{"SetInnerRML + Update",
 			[&](const String& rml) {
 				el->SetInnerRML(rml);
-			}
-		},
-		{
-			"Update (unmodified)",
-			[&](const String& /*rml*/) {
 				context->Update();
-			}
-		},
-		{
-			"Render",
-			[&](const String& /*rml*/) {
-				context->Render();
-			}
-		},
-		{
-			"SetInnerRML + Update",
-			[&](const String& rml) {
-				el->SetInnerRML(rml);
-				context->Update();
-			}
-		},
-		{
-			"SetInnerRML + Update + Render",
+			}},
+		{"SetInnerRML + Update + Render",
 			[&](const String& rml) {
 				el->SetInnerRML(rml);
 				context->Update();
 				context->Render();
-			}
-		},
+			}},
 	};
 
 	for (auto& bench_def : bench_list)
@@ -237,7 +217,7 @@ TEST_CASE("element.asymptotic_complexity")
 		bench.relative(true);
 
 		// Running the benchmark multiple times, with different number of rows.
-		for (const int num_rows : { 1, 2, 5, 10, 20, 50, 100, 200, 500 })
+		for (const int num_rows : {1, 2, 5, 10, 20, 50, 100, 200, 500})
 		{
 			const String rml = GenerateRml(num_rows);
 
@@ -245,9 +225,7 @@ TEST_CASE("element.asymptotic_complexity")
 			context->Update();
 			context->Render();
 
-			bench.complexityN(num_rows).run(bench_def.title, [&]() {
-				bench_def.run(rml);
-			});
+			bench.complexityN(num_rows).run(bench_def.title, [&]() { bench_def.run(rml); });
 		}
 
 #if defined(RMLUI_BENCHMARKS_SHOW_COMPLEXITY) || 0

+ 12 - 1
Tests/Source/Benchmarks/Selectors.cpp

@@ -98,7 +98,8 @@ enum SelectorFlags {
 	ID = 1 << 1,
 	CLASS = 1 << 2,
 	PSEUDO_CLASS = 1 << 3,
-	NUM_COMBINATIONS = 1 << 4,
+	ATTRIBUTE = 1 << 4,
+	NUM_COMBINATIONS = 1 << 5,
 };
 
 static String GenerateRCSS(SelectorFlags selectors, const String& complex_selector, String& out_rule_name)
@@ -131,6 +132,8 @@ static String GenerateRCSS(SelectorFlags selectors, const String& complex_select
 				rule += '.' + name;
 			if (selectors & PSEUDO_CLASS)
 				rule += ':' + name;
+			if (selectors & ATTRIBUTE)
+				rule += '[' + name + ']';
 		}
 
 		return rule;
@@ -221,6 +224,7 @@ TEST_CASE("Selectors")
 	bench.relative(true);
 
 	const Vector<String> complex_selectors = {
+		"*",
 		"div",
 		"div div",
 		"div > div",
@@ -232,6 +236,13 @@ TEST_CASE("Selectors")
 		":nth-child(2n+3) div",
 		":nth-of-type(2n+3) div",
 		":not(div) div",
+		"[class] div",
+		"[class=col] div",
+		"[class~=col] div",
+		"[class|=col] div",
+		"[class^=col] div",
+		"[class$=col] div",
+		"[class*=col] div",
 	};
 
 	for (int i = 0; i < NUM_COMBINATIONS + (int)complex_selectors.size(); i++)

+ 101 - 9
Tests/Source/UnitTests/Selectors.cpp

@@ -59,23 +59,26 @@ static const String doc_end = R"(
 	<div  id="P" class="parent">
 		<h1 id="A"/>
 		Some text
-		<p  id="B"/>
+		<p  id="B" unit="m"/>
 		<p  id="C"/>
 		<p  id="D"> <span id="D0">  </span><span id="D1">Text</span> </p>
 		<h3 id="E"/>
-		<p  id="F"> <span id="F0"/> </p>
-		<p  id="G"/>
-		<p  id="H" class="hello"/>
+		<p  id="F"> <span id="F0" class="hello-world"/> </p>
+		<p  id="G" class/>
+		<p  id="H" class="world hello"/>
 	</div>
 	<input id="I" type="checkbox" checked/>
 </body>
 </rml>
 )";
 
-enum class SelectorOp { None, RemoveElementsByIds, InsertElementBefore, RemoveClasses, RemoveChecked };
+enum class SelectorOp { None, RemoveElementsByIds, InsertElementBefore, RemoveClasses, RemoveId, RemoveChecked, RemoveAttributeUnit, SetHover };
 
 struct QuerySelector {
-	QuerySelector(String selector, String expected_ids) : selector(std::move(selector)), expected_ids(std::move(expected_ids)) {}
+	QuerySelector(String selector, String expected_ids, int expect_num_warnings = 0, int expect_num_query_warnings = 0) :
+		selector(std::move(selector)), expected_ids(std::move(expected_ids)), expect_num_warnings(expect_num_warnings),
+		expect_num_query_warnings(expect_num_query_warnings)
+	{}
 	QuerySelector(String selector, String expected_ids, SelectorOp operation, String operation_argument, String expected_ids_after_operation) :
 		selector(std::move(selector)), expected_ids(std::move(expected_ids)), operation(operation), operation_argument(std::move(operation_argument)),
 		expected_ids_after_operation(std::move(expected_ids_after_operation))
@@ -83,6 +86,9 @@ struct QuerySelector {
 	String selector;
 	String expected_ids;
 
+	// If this rule should fail to parse or otherwise produce warnings, set these to non-zero values.
+	int expect_num_warnings = 0, expect_num_query_warnings = 0;
+
 	// Optionally also test the selector after dynamically making a structural operation on the document.
 	SelectorOp operation = SelectorOp::None;
 	String operation_argument;
@@ -94,7 +100,7 @@ static const Vector<QuerySelector> query_selectors =
 {
 	{ "span",                        "Y D0 D1 F0" },
 	{ ".hello",                      "X Z H" },
-	{ ".hello.world",                "Z" },
+	{ ".hello.world",                "Z H" },
 	{ "div.hello",                   "X Z" },
 	{ "body .hello",                 "X Z H" },
 	{ "body>.hello",                 "X Z" },
@@ -102,6 +108,52 @@ 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", "" },
+
+	{ "*",                           "X Y Z P A B C D D0 D1 E F F0 G H I" },
+	{ "*span",                       "Y D0 D1 F0" },
+	{ "*.hello",                     "X Z H" },
+	{ "*:checked",                   "I" },
+	
+	{ "p[unit='m']",                 "B" },
+	{ "p[unit=\"m\"]",               "B" },
+	{ "[class]",                     "X Y Z P F0 G H" },
+	{ "[class=hello]",               "X" },
+	{ "[class=]",                    "G" },
+	{ "[class='']",                  "G" },
+	{ "[class=\"\"]",                "G" },
+	{ "[class~=hello]",              "X Z H" },
+	{ "[class~=ello]",               "" },
+	{ "[class|=hello]",              "F0" },
+	{ "[class^=hello]",              "X Z F0" },
+	{ "[class^=]",                   "X Y Z P F0 G H" },
+	{ "[class$=hello]",              "X H" },
+	{ "[class*=hello]",              "X Z F0 H" },
+	{ "[class*=ello]",               "X Z F0 H" },
+	
+	{ "[class~=hello].world",         "Z H" },
+	{ "*[class~=hello].world",        "Z H" },
+	{ ".world[class~=hello]",         "Z H" },
+	{ "[class~=hello][class~=world]", "Z H" },
+
+	{ "[class=hello world]",          "Z" },
+	{ "[class='hello world']",        "Z" },
+	{ "[class=\"hello world\"]",      "Z" },
+
+	{ "[invalid",                     "", 1, 4 },
+	{ "[]",                           "", 1, 4 },
+	{ "[x=Rule{What}]",               "", 2, 0 },
+	{ "[x=Hello,world]",              "", 1, 2 }, 
+	// The next ones are valid in CSS but we currently don't bother handling them, just make sure we don't crash.
+	{ "[x='Rule{What}']",             "", 2, 0 },
+	{ "[x='Hello,world']",            "", 1, 2 }, 
+
+	{ "#X[class=hello]",              "X" },
+	{ "[class=hello]#X",              "X" },
+	{ "#Y[class=hello]",              "" },
+	{ "div[class=hello]",             "X" },
+	{ "[class=hello]div",             "X" },
+	{ "span[class=hello]",            "" },
+	
 	{ ".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 +176,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" },
+	
+	{ ".hello.world, #P span, #I",   "Z D0 D1 F0 H 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,12 +200,28 @@ 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", ""  },
+
+	{ "p[unit=m]",                   "B",               SelectorOp::RemoveAttributeUnit,  "B", ""  },
+	{ "p[unit=m] + *",               "C",               SelectorOp::RemoveAttributeUnit,  "B", ""  },
+
+	{ "body > * #D0",                "D0" },
+	{ "#E + * ~ *",                  "G H" },
+	{ "#B + * ~ #G",                 "G" },
+	{ "body > :nth-child(4) span:first-child",  "D0 F0", SelectorOp::RemoveElementsByIds,  "X",    "" },
 };
 
 struct ClosestSelector {
@@ -241,10 +316,13 @@ TEST_CASE("Selectors")
 	{
 		for (const QuerySelector& selector : query_selectors)
 		{
+			TestsShell::SetNumExpectedWarnings(selector.expect_num_warnings);
 			const String selector_css = selector.selector + " { drag: drag; } ";
 			const String document_string = doc_begin + selector_css + doc_end;
 			ElementDocument* document = context->LoadDocumentFromMemory(document_string);
-			REQUIRE(document);
+
+			// Update the context to settle any dirty state.
+			context->Update();
 
 			String matching_ids;
 			GetMatchingIds(matching_ids, document);
@@ -272,6 +350,18 @@ TEST_CASE("Selectors")
 					document->GetElementById(selector.operation_argument)->RemoveAttribute("checked");
 					operation_str = "RemoveChecked";
 					break;
+				case SelectorOp::RemoveId:
+					document->GetElementById(selector.operation_argument)->SetId("");
+					operation_str = "RemoveId";
+					break;
+				case SelectorOp::RemoveAttributeUnit:
+					document->GetElementById(selector.operation_argument)->RemoveAttribute("unit");
+					operation_str = "RemoveAttributeUnit";
+					break;
+				case SelectorOp::SetHover:
+					document->GetElementById(selector.operation_argument)->SetPseudoClass("hover", true);
+					operation_str = "SetHover";
+					break;
 				case SelectorOp::None:
 					break;
 				}
@@ -295,6 +385,8 @@ TEST_CASE("Selectors")
 
 		for (const QuerySelector& selector : query_selectors)
 		{
+			TestsShell::SetNumExpectedWarnings(selector.expect_num_query_warnings);
+
 			ElementList elements;
 			document->QuerySelectorAll(elements, selector.selector);
 			String matching_ids = ElementListToIds(elements);