On Sun, 2004-04-18 at 23:57, Not Zed wrote:
> Shouldn't the size be specified as like
> E_ICON_FACTORY_ICON
> E_ICON_FACTORY_MENU
> etc.
> rather than
> 16
> 24
> 32?
>
> I don't think it should specify the exact pixel sizes.
Yeah, OK, that makes sense.
> - iconpart = em_format_html_file_part((EMFormatHTML *)emf,
> "image/png", EVOLUTION_ICONSDIR,
> -
> comp&&comp[0]?"flag-for-followup-done-16.png":"flag-for-followup-16.png");
> - if (iconpart) {
> - char *classid;
> -
> - classid =
> g_strdup_printf("icon:///em-format-html-display/%s/%s",
> emf->part_id->str, comp&&comp[0]?"comp":"uncomp");
> - camel_stream_printf(stream, "<td align=\"left\"><img
> src=\"%s\"></td>", classid);
> - (void)em_format_add_puri(emf, sizeof(EMFormatPURI),
> classid, iconpart, efhd_write_image);
> - g_free(classid);
> - camel_object_unref(iconpart);
> + iconpath = e_icon_factory_get_icon_filename (comp && comp[0] ?
> "stock_flag-for-followup-done" : "stock_flag-for-followup", 16);
> + if (iconpath) {
> + CamelMimePart *iconpart;
> +
> + iconpart = em_format_html_file_part((EMFormatHTML
> *)emf, "image/png", iconpath);
> + g_free (iconpath);
> + if (iconpart) {
> + char *classid;
> +
> + classid =
> g_strdup_printf("icon:///em-format-html-display/%s/%s",
> emf->part_id->str, comp&&comp[0]?"comp":"uncomp");
> + camel_stream_printf(stream, "<td
> align=\"left\"><img src=\"%s\"></td>", classid);
> + (void)em_format_add_puri(emf,
> sizeof(EMFormatPURI), classid, iconpart, efhd_write_image);
> + g_free(classid);
> + camel_object_unref(iconpart);
> + }
> }
>
>
> For above:
> 1. we need to display something if we get no filename. does
> e-icon-theme fallback to an unthemed icon if it can't find one? do we
> have to check for null filename at all?
> 2. how can we be sure that the icon has a filename, and that it is a
> png?
> 3. in the sign icon stuff later on we don't check filename isn't null.
If a NULL filename is passed or the icon cannot be found, a blank icon
is provided. This keeps everything looking normal and allows
e_icon_factory to promise that it will always return a pixbuf of the
right size.
You are right though, that the icon may not be a png. To fix that, we
could use gnome-vfs to figure out the mime type. But do we have to
specify a mime type there?
> - char *filename;
> + gchar *basename;
>
> use char instead of gchar everywhere in the mail code.
OK.
> The new code in e-icon-theme.c uses
> }
> else {
> it should be:
> } else {
OK.
> Also, why are we using pixbufs? Pixmaps would be far more efficient
> for most cases.
Pixbufs are the standard object for the icon theme. I also thought that
pixmaps were somewhat deprecated.
> Allso there is some inconsitency with hyphens. Should they all be -
> or _ or why the different mix?
The different mix is right. Icons all use hyphens except stock icons
which use one underscore after the word stock.
> + icon_list = e_icon_factory_get_icon_list
> ("stock_mail-send-receive");
> + if (icon_list) {
> + gtk_window_set_icon_list (GTK_WINDOW (gd), icon_list);
> + g_list_foreach (icon_list, (GFunc) g_object_unref,
> NULL);
> + g_list_free (icon_list);
> + }
>
> this code is repeated a lot, should there be a free_icon_list call?
It's possible that there should be a convenience function in
e_icon_factory for setting a window's icon. That is the big use case
prompting the lists of icons. I'll write one up.
I'll start working on a patch to fix these issues.
-mt
Attachment:
signature.asc
Description: This is a digitally signed message part