Improve Screenshot validation

src/as-validator-issue-tag.h

@@ -204,6 +204,41 @@ AsValidatorIssueTag as_validator_issue_tag_list[] =  {
 	  N_("The update-contact does not appear to be a valid email address (escaping of `@` is only allowed as `_at_` or `_AT_`).")
 	},
 
+	{ "screenshot-invalid-width",
+	  AS_ISSUE_SEVERITY_ERROR,
+	  N_("The width property must be a positive integer")
+	},
+
+	{ "screenshot-invalid-height",
+	  AS_ISSUE_SEVERITY_ERROR,
+	  N_("The width property must be a integer")
+	},
+
+	{ "screenshot-image-invalid-type",
+	  AS_ISSUE_SEVERITY_ERROR,
+	  N_("The type of the image is neither `source` or `thumbnail`")
+	},
+
+	{ "screenshot-image-missing-width",
+	  AS_ISSUE_SEVERITY_WARNING,
+	  N_("The `width` property must be present if the type is `thumbnail`")
+	},
+
+	{ "screenshot-image-missing-height",
+	  AS_ISSUE_SEVERITY_WARNING,
+	  N_("The `height` property must be present if the type is thumbnail")
+	},
+
+	{ "screenshot-image-source-duplicated",
+	  AS_ISSUE_SEVERITY_ERROR,
+	  N_("The 'source' type can only be used once for a image and a language")
+	},
+
+	{ "screenshot-image-source-missing",
+	  AS_ISSUE_SEVERITY_ERROR,
+	  N_("A screenshot must have at least one image of type `source`")
+	},
+
 	{ "screenshot-image-not-found",
 	  AS_ISSUE_SEVERITY_WARNING,
 	  N_("Unable to reach the screenshot image on its remote location - does the image exist?")
@@ -273,6 +308,11 @@ AsValidatorIssueTag as_validator_issue_tag_list[] =  {
 	  N_("The default screenshot of a software component must not be a video. Use a static image as default screenshot and set the video as a secondary screenshot.")
 	},
 
+	{ "screenshot-default-missing",
+	  AS_ISSUE_SEVERITY_ERROR,
+	  N_("A default screenshot is missing.")
+	},
+
 	{ "relation-invalid-tag",
 	  AS_ISSUE_SEVERITY_WARNING,
 	  N_("Found an unknown tag in a requires/recommends group. This is likely an error, because a component relation of this type is unknown.")

src/as-validator.c

@@ -1228,20 +1228,27 @@ as_validator_check_tags (AsValidator *validator, xmlNode *node)
 static void
 as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsComponent *cpt)
 {
+	gboolean default_screenshot_found = FALSE;
 	for (xmlNode *iter = node->children; iter != NULL; iter = iter->next) {
 		xmlNode *iter2;
 		gboolean image_found = FALSE;
 		gboolean video_found = FALSE;
 		gboolean caption_found = FALSE;
 		gboolean default_screenshot = FALSE;
 		g_autofree gchar *scr_kind_str = NULL;
+		gboolean default_source_found = FALSE;
+		g_autoptr(GHashTable) known_source_lang = NULL;
+
+		known_source_lang = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
 
 		if (iter->type != XML_ELEMENT_NODE)
 			continue;
 
 		scr_kind_str = as_xml_get_prop_value (iter, "type");
-		if (g_strcmp0 (scr_kind_str, "default") == 0)
+		if (as_screenshot_kind_from_string (scr_kind_str) == AS_SCREENSHOT_KIND_DEFAULT) {
+			default_screenshot_found = TRUE;
 			default_screenshot = TRUE;
+		}
 
 		if (g_strcmp0 ((const gchar*) iter->name, "screenshot") != 0) {
 			as_validator_add_issue (validator, iter,
@@ -1253,14 +1260,74 @@ as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsCompone
 		}
 
 		for (iter2 = iter->children; iter2 != NULL; iter2 = iter2->next) {
+			g_autofree gchar *screenshot_width = NULL;
+			g_autofree gchar *screenshot_height = NULL;
+
 			const gchar *node_name = (const gchar*) iter2->name;
 			if (iter2->type != XML_ELEMENT_NODE)
 				continue;
 
+
+			screenshot_width = as_xml_get_prop_value (iter2, "width");
+			if (screenshot_width != NULL && !as_str_verify_integer (screenshot_width, 1, G_MAXINT64))
+				as_validator_add_issue (validator, iter2,
+					"screenshot-invalid-width",
+					screenshot_width);
+
+			screenshot_height = as_xml_get_prop_value (iter2, "height");
+			if (screenshot_height != NULL && !as_str_verify_integer (screenshot_height, 1, G_MAXINT64))
+				as_validator_add_issue (validator, iter2,
+					"screenshot-invalid-height",
+					screenshot_height);
+
 			if (g_strcmp0 (node_name, "image") == 0) {
 				g_autofree gchar *image_url = as_xml_get_node_value (iter2);
+        g_autofree gchar *image_type = NULL;
 
 				image_found = TRUE;
+
+				image_type = as_xml_get_prop_value (iter2, "type");
+				if (image_type == NULL)
+					image_type = "source";
+
+				if (g_strcmp0 (image_type, "source") != 0 && g_strcmp0 (image_type, "thumbnail") != 0)
+					as_validator_add_issue (validator, iter2,
+								"screenshot-image-invalid-type",
+								image_type);
+
+				if (screenshot_width == NULL && g_strcmp0 (image_type, "thumbnail") == 0)
+					as_validator_add_issue (validator, iter2,
+						"screenshot-image-missing-width",
+						NULL);
+
+				if (screenshot_height == NULL && g_strcmp0 (image_type, "thumbnail") == 0)
+					as_validator_add_issue (validator, iter2,
+						"screenshot-image-missing-height",
+						NULL);
+
+				if (g_strcmp0(image_type, "source") == 0) {
+					gchar *image_lang = NULL;
+
+					image_lang = as_xml_get_prop_value (iter2, "lang");
+
+					if (image_lang == NULL) {
+						if (default_source_found)
+							as_validator_add_issue (validator, iter2,
+								"screenshot-image-source-duplicated",
+								NULL);
+
+						default_source_found = TRUE;
+					}
+					else {
+						if (g_hash_table_contains (known_source_lang, image_lang))
+							as_validator_add_issue (validator, iter2,
+							"screenshot-image-source-duplicated",
+							image_lang);
+
+						g_hash_table_add (known_source_lang, image_lang);
+					}
+				}
+
 				if (!as_validate_is_url (image_url)) {
 					as_validator_add_issue (validator, iter2,
 								"web-url-expected",
@@ -1351,10 +1418,15 @@ as_validator_check_screenshots (AsValidator *validator, xmlNode *node, AsCompone
 			as_validator_add_issue (validator, iter, "screenshot-no-media", NULL);
 		else if (image_found && video_found)
 			as_validator_add_issue (validator, iter, "screenshot-mixed-images-videos", NULL);
+		else if (image_found && !default_source_found)
+			as_validator_add_issue (validator, iter, "screenshot-image-source-missing", NULL);
 
 		if (!caption_found)
 			as_validator_add_issue (validator, iter, "screenshot-no-caption", NULL);
 	}
+
+	if (!default_screenshot_found)
+		as_validator_add_issue (validator, node, "screenshot-default-missing", NULL);
 }
 
 /**