WIP: PropertyIds. Need to figure out how to deal with property groups. Possible solutions:

1. Remove them entirely, replace affected properties by a new predefined property
2. Add separate property groups to the stylesheetnodes and find them from element definition
3. Register the group properties as new property ids the moment they are encountered

Include/Rocket/Core/StyleSheetSpecification.h

@@ -109,6 +109,8 @@ public:
 	static const String& GetPropertyName(PropertyId id);
 	static const String& GetShorthandName(ShorthandId id);
 
+	static std::vector<PropertyId> GetShorthandUnderlyingProperties(ShorthandId id);
+
 private:
 	StyleSheetSpecification();
 	~StyleSheetSpecification();

Source/Core/Context.cpp

@@ -1115,9 +1115,9 @@ void Context::CreateDragClone(Element* element)
 
 	// Set all the required properties and pseudo-classes on the clone.
 	drag_clone->SetPseudoClass("drag", true);
-	drag_clone->SetProperty("position", Property(Style::Position::Absolute));
-	drag_clone->SetProperty("left", Property(element->GetAbsoluteLeft() - element->GetBox().GetEdge(Box::MARGIN, Box::LEFT) - mouse_position.x, Property::PX));
-	drag_clone->SetProperty("top", Property(element->GetAbsoluteTop() - element->GetBox().GetEdge(Box::MARGIN, Box::TOP) - mouse_position.y, Property::PX));
+	drag_clone->SetProperty(PropertyId::Position, Property(Style::Position::Absolute));
+	drag_clone->SetProperty(PropertyId::Left, Property(element->GetAbsoluteLeft() - element->GetBox().GetEdge(Box::MARGIN, Box::LEFT) - mouse_position.x, Property::PX));
+	drag_clone->SetProperty(PropertyId::Top, Property(element->GetAbsoluteTop() - element->GetBox().GetEdge(Box::MARGIN, Box::TOP) - mouse_position.y, Property::PX));
 }
 
 // Releases the drag clone, if one exists.

Source/Core/ElementDocument.cpp

@@ -55,7 +55,7 @@ ElementDocument::ElementDocument(const String& tag) : Element(tag)
 	ForceLocalStackingContext();
 	SetOwnerDocument(this);
 
-	SetProperty(POSITION, Property(Style::Position::Absolute));
+	SetProperty(PropertyId::Position, Property(Style::Position::Absolute));
 }
 
 ElementDocument::~ElementDocument()
@@ -150,7 +150,7 @@ void ElementDocument::ProcessHeader(const DocumentHeader* document_header)
 	}
 
 	// Hide this document.
-	SetProperty(VISIBILITY, Property(Style::Visibility::Hidden));
+	SetProperty(PropertyId::Visibility, Property(Style::Visibility::Hidden));
 }
 
 // Returns the document's context.
@@ -220,7 +220,7 @@ void ElementDocument::Show(int focus_flags)
 	modal = (focus_flags & MODAL) > 0;
 
 	// Set to visible and switch focus if necessary
-	SetProperty(VISIBILITY, Property(Style::Visibility::Visible));
+	SetProperty(PropertyId::Visibility, Property(Style::Visibility::Visible));
 	
 	// We should update the document now, otherwise the focusing methods below do not think we are visible
 	// If this turns out to be slow, the more performant approach is just to compute the new visibility property
@@ -237,7 +237,7 @@ void ElementDocument::Show(int focus_flags)
 
 void ElementDocument::Hide()
 {
-	SetProperty(VISIBILITY, Property(Style::Visibility::Hidden));
+	SetProperty(PropertyId::Visibility, Property(Style::Visibility::Hidden));
 
 	// We should update the document now, so that the focusing below will get the correct visibility
 	UpdateDocument();
@@ -391,13 +391,13 @@ void ElementDocument::OnPropertyChange(const PropertyNameList& changed_propertie
 	Element::OnPropertyChange(changed_properties);
 
 	// If the document's font-size has been changed, we need to dirty all rem properties.
-	if (changed_properties.find(FONT_SIZE) != changed_properties.end())
+	if (changed_properties.find(PropertyId::FontSize) != changed_properties.end())
 		GetStyle()->DirtyRemProperties();
 
-	if (changed_properties.find(TOP) != changed_properties.end() ||
-		changed_properties.find(RIGHT) != changed_properties.end() ||
-		changed_properties.find(BOTTOM) != changed_properties.end() ||
-		changed_properties.find(LEFT) != changed_properties.end())
+	if (changed_properties.find(PropertyId::Top) != changed_properties.end() ||
+		changed_properties.find(PropertyId::Right) != changed_properties.end() ||
+		changed_properties.find(PropertyId::Bottom) != changed_properties.end() ||
+		changed_properties.find(PropertyId::Left) != changed_properties.end())
 		DirtyPosition();
 }
 

Source/Core/ElementHandle.cpp

@@ -38,7 +38,7 @@ namespace Core {
 ElementHandle::ElementHandle(const String& tag) : Element(tag), drag_start(0, 0)
 {
 	// Make sure we can be dragged!
-	SetProperty(DRAG, Property(Style::Drag::Drag));
+	SetProperty(PropertyId::Drag, Property(Style::Drag::Drag));
 
 	move_target = NULL;
 	size_target = NULL;
@@ -107,8 +107,8 @@ void ElementHandle::ProcessDefaultAction(Event& event)
 			// Update the move and size objects
 			if (move_target)
 			{
-				move_target->SetProperty(LEFT, Property(Math::RealToInteger(move_original_position.x + x), Property::PX));
-				move_target->SetProperty(TOP, Property(Math::RealToInteger(move_original_position.y + y), Property::PX));
+				move_target->SetProperty(PropertyId::Left, Property(Math::RealToInteger(move_original_position.x + x), Property::PX));
+				move_target->SetProperty(PropertyId::Top, Property(Math::RealToInteger(move_original_position.y + y), Property::PX));
 			}
 
 			if (size_target)
@@ -118,19 +118,19 @@ void ElementHandle::ProcessDefaultAction(Event& event)
 
 				// Check if we have auto-margins; if so, they have to be set to the current margins.
 				if (computed.margin_top.type == Margin::Auto)
-					size_target->SetProperty(MARGIN_TOP, Property((float) Math::RealToInteger(size_target->GetBox().GetEdge(Box::MARGIN, Box::TOP)), Property::PX));
+					size_target->SetProperty(PropertyId::MarginTop, Property((float) Math::RealToInteger(size_target->GetBox().GetEdge(Box::MARGIN, Box::TOP)), Property::PX));
 				if (computed.margin_right.type == Margin::Auto)
-					size_target->SetProperty(MARGIN_RIGHT, Property((float) Math::RealToInteger(size_target->GetBox().GetEdge(Box::MARGIN, Box::RIGHT)), Property::PX));
+					size_target->SetProperty(PropertyId::MarginRight, Property((float) Math::RealToInteger(size_target->GetBox().GetEdge(Box::MARGIN, Box::RIGHT)), Property::PX));
 				if (computed.margin_bottom.type == Margin::Auto)
-					size_target->SetProperty(MARGIN_BOTTOM, Property((float) Math::RealToInteger(size_target->GetBox().GetEdge(Box::MARGIN, Box::BOTTOM)), Property::PX));
+					size_target->SetProperty(PropertyId::MarginBottom, Property((float) Math::RealToInteger(size_target->GetBox().GetEdge(Box::MARGIN, Box::BOTTOM)), Property::PX));
 				if (computed.margin_left.type == Margin::Auto)
-					size_target->SetProperty(MARGIN_LEFT, Property((float) Math::RealToInteger(size_target->GetBox().GetEdge(Box::MARGIN, Box::LEFT)), Property::PX));
+					size_target->SetProperty(PropertyId::MarginLeft, Property((float) Math::RealToInteger(size_target->GetBox().GetEdge(Box::MARGIN, Box::LEFT)), Property::PX));
 
 				int new_x = Math::RealToInteger(size_original_size.x + x);
 				int new_y = Math::RealToInteger(size_original_size.y + y);
 
-				size_target->SetProperty(WIDTH, Property(Math::Max< float >((float) new_x, 0), Property::PX));
-				size_target->SetProperty(HEIGHT, Property(Math::Max< float >((float) new_y, 0), Property::PX));
+				size_target->SetProperty(PropertyId::Width, Property(Math::Max< float >((float) new_x, 0), Property::PX));
+				size_target->SetProperty(PropertyId::Height, Property(Math::Max< float >((float) new_y, 0), Property::PX));
 			}
 
 			Dictionary parameters;

Source/Core/ElementImage.cpp

@@ -160,8 +160,8 @@ void ElementImage::OnPropertyChange(const PropertyNameList& changed_properties)
 {
     Element::OnPropertyChange(changed_properties);
 
-    if (changed_properties.find(IMAGE_COLOR) != changed_properties.end() ||
-        changed_properties.find(OPACITY) != changed_properties.end()) {
+    if (changed_properties.find(PropertyId::ImageColor) != changed_properties.end() ||
+        changed_properties.find(PropertyId::Opacity) != changed_properties.end()) {
         GenerateGeometry();
     }
 }

Source/Core/ElementScroll.cpp

@@ -81,7 +81,7 @@ void ElementScroll::EnableScrollbar(Orientation orientation, float element_width
 	if (!scrollbars[orientation].enabled)
 	{
 		CreateScrollbar(orientation);
-		scrollbars[orientation].element->SetProperty(VISIBILITY, Property(Style::Visibility::Visible));
+		scrollbars[orientation].element->SetProperty(PropertyId::Visibility, Property(Style::Visibility::Visible));
 		scrollbars[orientation].enabled = true;
 	}
 
@@ -107,7 +107,7 @@ void ElementScroll::DisableScrollbar(Orientation orientation)
 {
 	if (scrollbars[orientation].enabled)
 	{
-		scrollbars[orientation].element->SetProperty(VISIBILITY, Property(Style::Visibility::Hidden));
+		scrollbars[orientation].element->SetProperty(PropertyId::Visibility, Property(Style::Visibility::Hidden));
 		scrollbars[orientation].enabled = false;
 	}
 }
@@ -217,12 +217,12 @@ void ElementScroll::FormatScrollbars()
 		corner->SetBox(corner_box);
 		corner->SetOffset(containing_block - Vector2f(scrollbars[VERTICAL].size, scrollbars[HORIZONTAL].size), element, true);
 
-		corner->SetProperty(VISIBILITY, Property(Style::Visibility::Visible));
+		corner->SetProperty(PropertyId::Visibility, Property(Style::Visibility::Visible));
 	}
 	else
 	{
 		if (corner != NULL)
-			corner->SetProperty(VISIBILITY, Property(Style::Visibility::Hidden));
+			corner->SetProperty(PropertyId::Visibility, Property(Style::Visibility::Hidden));
 	}
 }
 
@@ -235,7 +235,7 @@ bool ElementScroll::CreateScrollbar(Orientation orientation)
 		return true;
 
 	scrollbars[orientation].element = Factory::InstanceElement(element, "*", orientation == VERTICAL ? "scrollbarvertical" : "scrollbarhorizontal", XMLAttributes());
-	scrollbars[orientation].element->SetProperty(CLIP, Property(1, Property::NUMBER));
+	scrollbars[orientation].element->SetProperty(PropertyId::Clip, Property(1, Property::NUMBER));
 
 	scrollbars[orientation].widget = new WidgetSliderScroll(scrollbars[orientation].element);
 	scrollbars[orientation].widget->Initialise(orientation == VERTICAL ? WidgetSlider::VERTICAL : WidgetSlider::HORIZONTAL);

Source/Core/ElementStyle.cpp

@@ -102,7 +102,7 @@ const Property* ElementStyle::GetProperty(PropertyId id, Element* element, Prope
 		return local_property;
 
 	// Fetch the property specification.
-	const PropertyDefinition* property = StyleSheetSpecification::GetProperty(name);
+	const PropertyDefinition* property = StyleSheetSpecification::GetProperty(id);
 	if (property == NULL)
 		return NULL;
 
@@ -112,7 +112,7 @@ const Property* ElementStyle::GetProperty(PropertyId id, Element* element, Prope
 		Element* parent = element->GetParentNode();
 		while (parent != NULL)
 		{
-			const Property* parent_property = parent->GetStyle()->GetLocalProperty(name);
+			const Property* parent_property = parent->GetStyle()->GetLocalProperty(id);
 			if (parent_property)
 				return parent_property;
 

Source/Core/ElementTextDefault.cpp

@@ -290,8 +290,8 @@ void ElementTextDefault::OnPropertyChange(const PropertyNameList& changed_proper
 	bool font_face_changed = false;
 	auto& computed = GetComputedValues();
 
-	if (changed_properties.find(COLOR) != changed_properties.end() || 
-		changed_properties.find(OPACITY) != changed_properties.end())
+	if (changed_properties.find(PropertyId::Color) != changed_properties.end() ||
+		changed_properties.find(PropertyId::Opacity) != changed_properties.end())
 	{
 		// Fetch our (potentially) new colour.
 		Colourb new_colour = computed.color;
@@ -302,11 +302,11 @@ void ElementTextDefault::OnPropertyChange(const PropertyNameList& changed_proper
 			colour = new_colour;
 	}
 
-	if (changed_properties.find(FONT_FAMILY) != changed_properties.end() ||
-		changed_properties.find(FONT_CHARSET) != changed_properties.end() ||
-		changed_properties.find(FONT_WEIGHT) != changed_properties.end() ||
-		changed_properties.find(FONT_STYLE) != changed_properties.end() ||
-		changed_properties.find(FONT_SIZE) != changed_properties.end())
+	if (changed_properties.find(PropertyId::FontFamily) != changed_properties.end() ||
+		changed_properties.find(PropertyId::FontCharset) != changed_properties.end() ||
+		changed_properties.find(PropertyId::FontWeight) != changed_properties.end() ||
+		changed_properties.find(PropertyId::FontStyle) != changed_properties.end() ||
+		changed_properties.find(PropertyId::FontSize) != changed_properties.end())
 	{
 		font_face_changed = true;
 
@@ -314,7 +314,7 @@ void ElementTextDefault::OnPropertyChange(const PropertyNameList& changed_proper
 		font_dirty = true;
 	}
 
-	if (changed_properties.find(TEXT_DECORATION) != changed_properties.end())
+	if (changed_properties.find(PropertyId::TextDecoration) != changed_properties.end())
 	{
 		decoration_property = (int)computed.text_decoration;
 		if (decoration_property != TEXT_DECORATION_NONE)

Source/Core/PropertyParserAnimation.cpp

@@ -232,7 +232,7 @@ static bool ParseTransition(Property & property, const StringList& transition_va
 	{
 
 		Transition transition;
-		StringList target_property_names;
+		PropertyNameList target_property_names;
 
 		StringList arguments;
 		StringUtilities::ExpandString(arguments, single_transition_value, ' ');
@@ -263,7 +263,6 @@ static bool ParseTransition(Property & property, const StringList& transition_va
 					if (transition_list.transitions.size() > 0) // The all keyword can not be part of multiple definitions
 						return false;
 					transition_list.all = true;
-					target_property_names.push_back("all");
 				}
 				else if (it->second.type == Keyword::TWEEN)
 				{
@@ -312,14 +311,13 @@ static bool ParseTransition(Property & property, const StringList& transition_va
 					// Must be a property name or shorthand, expand now
 					if (auto shorthand = StyleSheetSpecification::GetShorthand(argument))
 					{
-						// For shorthands, add each underlying property separately
-						for (const auto& property : shorthand->properties)
-							target_property_names.push_back(property.first);
+						auto underlying_properties = StyleSheetSpecification::GetShorthandUnderlyingProperties(shorthand->id);
+						target_property_names.insert(underlying_properties.begin(), underlying_properties.end());
 					}
 					else if (auto definition = StyleSheetSpecification::GetProperty(argument))
 					{
 						// Single property
-						target_property_names.push_back(argument);
+						target_property_names.insert(definition->GetId());
 					}
 					else
 					{
@@ -339,7 +337,7 @@ static bool ParseTransition(Property & property, const StringList& transition_va
 
 		for (const auto& property_name : target_property_names)
 		{
-			transition.name = property_name;
+			transition.id = property_name;
 			transition_list.transitions.push_back(transition);
 		}
 	}

Source/Core/PropertySpecification.cpp

@@ -63,8 +63,12 @@ PropertyDefinition& PropertySpecification::RegisterProperty(PropertyId id, const
 
 	if (index < properties.size())
 	{
-		// Delete any existing property.
-		delete properties[index];
+		// We don't want to owerwrite an existing entry.
+		if (properties[index])
+		{
+			Log::Message(Log::LT_ERROR, "While registering property '%s': The property is already registered, ignoring.", StyleSheetSpecification::GetPropertyName(id).c_str());
+			return *properties[index];
+		}
 	}
 	else
 	{
@@ -164,8 +168,12 @@ bool PropertySpecification::RegisterShorthand(ShorthandId id, const String& prop
 
 	if (index < shorthands.size())
 	{
-		// Delete any existing property.
-		delete shorthands[index];
+		// We don't want to owerwrite an existing entry.
+		if (shorthands[index])
+		{
+			Log::Message(Log::LT_ERROR, "While registering shorthand '%s': The shorthand is already registered, ignoring.", StyleSheetSpecification::GetShorthandName(id).c_str());
+			return false;
+		}
 	}
 	else
 	{

Source/Core/StyleSheetParser.cpp

@@ -78,22 +78,22 @@ static void PostprocessKeyframes(KeyframesMap& keyframes_map)
 	{
 		Keyframes& keyframes = keyframes_pair.second;
 		auto& blocks = keyframes.blocks;
-		auto& property_names = keyframes.property_names;
+		auto& property_ids = keyframes.property_ids;
 
 		// Sort keyframes on selector value.
 		std::sort(blocks.begin(), blocks.end(), [](const KeyframeBlock& a, const KeyframeBlock& b) { return a.normalized_time < b.normalized_time; });
 
 		// Add all property names specified by any block
-		if(blocks.size() > 0) property_names.reserve(blocks.size() * blocks[0].properties.GetNumProperties());
+		if(blocks.size() > 0) property_ids.reserve(blocks.size() * blocks[0].properties.GetNumProperties());
 		for(auto& block : blocks)
 		{
 			for (auto& property : block.properties.GetProperties())
-				property_names.push_back(property.first);
+				property_ids.push_back(property.first);
 		}
 		// Remove duplicate property names
-		std::sort(property_names.begin(), property_names.end());
-		property_names.erase(std::unique(property_names.begin(), property_names.end()), property_names.end());
-		property_names.shrink_to_fit();
+		std::sort(property_ids.begin(), property_ids.end());
+		property_ids.erase(std::unique(property_ids.begin(), property_ids.end()), property_ids.end());
+		property_ids.shrink_to_fit();
 	}
 
 }

Source/Core/StyleSheetSpecification.cpp

@@ -34,6 +34,7 @@
 #include "PropertyParserKeyword.h"
 #include "PropertyParserString.h"
 #include "PropertyParserTransform.h"
+#include "PropertyShorthandDefinition.h"
 
 namespace Rocket {
 namespace Core {
@@ -272,6 +273,30 @@ const String& StyleSheetSpecification::GetShorthandName(ShorthandId id)
 	return shorthand_id_name_map.GetName(id);
 }
 
+std::vector<PropertyId> StyleSheetSpecification::GetShorthandUnderlyingProperties(ShorthandId id)
+{
+	std::vector<PropertyId> result;
+	const ShorthandDefinition* shorthand = instance->properties.GetShorthand(id);
+	if (!shorthand)
+		return result;
+
+	result.reserve(shorthand->items.size());
+	for (auto& item : shorthand->items)
+	{
+		if (item.type == ShorthandItemType::Property)
+		{
+			result.push_back(item.property_id);
+		}
+		else if (item.type == ShorthandItemType::Shorthand)
+		{
+			// When we have a shorthand pointing to another shorthands, call us recursively
+			std::vector<PropertyId> new_items = GetShorthandUnderlyingProperties(item.shorthand_id);
+			result.insert(result.end(), new_items.begin(), new_items.end());
+		}
+	}
+	return result;
+}
+
 // Registers Rocket's default parsers.
 void StyleSheetSpecification::RegisterDefaultParsers()
 {