GNOME.org
 

Re: [evolution-patches] Re: [Evolution-hackers] Mailing list actions bounty (2)


On Wed, 2003-12-03 at 20:57, Jorge Bernal wrote: 
Not Zed dijo:

>
>
> Ok a few things - i know this is still work in progress, so i'm not sure
> if some of the code is left over from your earlier attempts to get the
> menu's correct.
>

Most of the patch was useless code :)

I think I've followed all of your guidelines, so check it:

http://gente.amedias.org/koke/src/patches/patch-mailing-list-actions-4.diff.gz


Hey again Jorge,

Some more comments inline.

Also, Edgar brought up on evolution-hackers a discussion point on how
the UI might work (subject 'RFC: Mailing List Actions UI).  For
example this patch only does popups, whereas his patch does it off the
Actions menu (although it will probably do popups too when its fixed).
There seem to be good reasons for each approach.  Now we've got a UI
designer (Hey Tigert), perhaps he can throw in some input?

Thanks,
Michael


> Index: em-folder-view.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-folder-view.c,v
> retrieving revision 1.15
> diff -u -r1.15 em-folder-view.c
> --- em-folder-view.c 24 Nov 2003 22:35:33 -0000 1.15
> +++ em-folder-view.c 3 Dec 2003 09:54:33 -0000
> @@ -1779,7 +1779,18 @@
>  emfv_format_link_clicked(EMFormatHTMLDisplay *efhd, const char *uri, EMFolderView *emfv)
>  {
>  if (!strncasecmp (uri, "mailto:", 7)) {
> - em_utils_compose_new_message_with_mailto (uri);
> + if (strstr (uri, "List-Post") != NULL) {
> + CamelURL *curl;
> +
> + curl = camel_url_new(uri, NULL);
> + if (camel_url_get_param (curl, "List-Post") != NULL)
> + em_utils_compose_new_message_with_mailto (camel_url_get_param (curl, "List-Post"));
> + else
> + em_utils_compose_new_message_with_mailto (uri);
> + g_free (curl);

Need to camel_url_free on curl.

> + } else {
> + em_utils_compose_new_message_with_mailto (uri);
> + }
>  } else if (*uri == '#') {
>  gtk_html_jump_to_anchor (((EMFormatHTML *) efhd)->html, uri + 1);
>  } else if (!strncasecmp (uri, "thismessage:", 12)) {
> Index: em-format-html.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-format-html.c,v
> retrieving revision 1.15
> diff -u -r1.15 em-format-html.c
> --- em-format-html.c 24 Nov 2003 22:35:33 -0000 1.15
> +++ em-format-html.c 3 Dec 2003 09:54:40 -0000
> @@ -1363,7 +1363,7 @@
>  {
>  char *mhtml = NULL;
>  const char *fmt, *html;
> -
> +
>  if (value == NULL)
>  return;
> 
> @@ -1398,13 +1398,93 @@
>  static void
>  efh_format_address(EMFormat *emf, CamelStream *stream, const CamelInternetAddress *cia, const char *name, guint32 flags)
>  {
> - char *text;
> + char *text, *list, *head = NULL;
> + char *tail;
> + char *new_text, *img_str;
> + char *classid;
> + CamelMimePart *icon;
> + CamelURL *curl;
> +
> +#define efh ((EMFormatHTML *)emf)
> 
>  if (cia == NULL || !camel_internet_address_get(cia, 0, NULL, NULL))
>  return;
> 
> + list = g_strdup(camel_medium_get_header (emf->message, "List-Post"));
> + if (list != NULL)
> + list = g_strndup (strchr(list, '<')+1 , strchr (list, '>') - strchr (list, '<') - 1);
> +

there's no need to strdup list here.  you don't free it later anyway.
and you're still assuming the list is in proper format above.

>  text = camel_address_format((CamelAddress *)cia);
> - efh_format_text_header(emf, stream, name, text, flags | EM_FORMAT_HEADER_BOLD);
> +
> + if (list != NULL)
> + head = strstr (text, strstr(list, "mailto:")+7);
> + if (head != NULL) {
> + char *listp;
> +
> + tail = strchr (head, ' ');
> + if (tail == NULL)
> + tail = strchr (head, '>');
> + if (tail == NULL)
> + tail = strchr (head, ',');
> +
> + head = g_strndup (text, head-text);
> +
> + curl = camel_url_new (list, NULL);
> +
> +#define url_setp(x) \
> + listp = camel_medium_get_header (emf->message, x); \
> + if (listp != NULL) \
> + listp = em_utils_extract_url (listp); \
> + if (listp != NULL) \
> + camel_url_set_param (curl, x, listp);
> +
> + url_setp("List-Help");
> + url_setp("List-Subscribe");
> + url_setp("List-Unsubscribe");
> + url_setp("List-Post");
> + url_setp("List-Archive");
> +
> +#undef url_setp
> + g_free(listp);

^^ this whole block needs some work.

i suggest something like:
- write a function that gets the header value(s)
- use that to get list-post to setup the initial url
- set all the others using a loop and get the value using the getting function

although there are many other reasonable ways to do it too.

Note also that according to the rfc, you can have multiple values in
this, they may not all be mailto url's, and you're supposed to do the
first one you can handle in the client.

> + classid = "" ("image://mail-list-16.png");

> + img_str = g_strdup_printf ("<a href=''><img alt='%s' border='0' src=''>%s</a>",
> + camel_url_to_string (curl, 0),

camel_url_to_string() returns a value you need to free (this is
indicated by it returning 'const char *' rather than 'char *' for a
return value).

Also, html uses double quotes to quote, not ', doesn't it?

> + N_("Mailing list"),
> + classid,
> + strstr(list, "mailto:")+7);
> + camel_object_unref (curl);
> + icon = em_format_html_file_part (efh, "image/png", EVOLUTION_ICONSDIR, "mail-list-16.png");
> + if (icon) {
> + em_format_add_puri (emf, sizeof(EMFormatPURI), classid, icon, efh_write_image);
> + camel_object_unref(icon);
> + }
> +
> + g_free(list);
> + g_free(classid);
> + if (tail != NULL)
> + new_text = g_strdup_printf("%s%s%s",
> + camel_text_to_html(head, efh->text_html_flags, 0),
> + img_str,
> + camel_text_to_html(tail, efh->text_html_flags, 0));
> + else
> + new_text = g_strdup_printf("%s%s",
> + camel_text_to_html(head, efh->text_html_flags, 0),
> + img_str);
> +
> + g_free(head);
> + g_free(tail);
> + g_free(img_str);


^^ this lot seems awfully complex.  You can probably do it all in one
printf.  You should be taking care of the list parameters in a
different way than appending 'tail'.


> + text = g_strdup(new_text);
> +
> + efh_format_text_header(emf, stream, name, text, flags | EM_FORMAT_HEADER_BOLD | EM_FORMAT_HTML_HEADER_HTML);

you strdup text and then don't free it?  this would leak.

> + } else {
> + efh_format_text_header(emf, stream, name, text, flags | EM_FORMAT_HEADER_BOLD);
> + }
> +#undef efh
> +
> +
>  g_free(text);
>  }
> 
> Index: em-format-html-display.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-format-html-display.c,v
> retrieving revision 1.10
> diff -u -r1.10 em-format-html-display.c
> --- em-format-html-display.c 1 Dec 2003 04:42:52 -0000 1.10
> +++ em-format-html-display.c 3 Dec 2003 09:54:43 -0000
> @@ -552,8 +552,8 @@
> 
>  d(printf("popup button pressed\n"));
> 
> - if ( (url = "" != NULL
> -      || (url = "" 0)) != NULL) {
> + if ( (url = "" 0)) != NULL
> +      || (url = "" != NULL) {
>  EMFormatPURI *puri;
>  char *uri;

FWIW make sure this works properly with other features like
right-clicking on images and being able to save.

> Index: em-popup.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-popup.c,v
> retrieving revision 1.7
> diff -u -r1.7 em-popup.c
> --- em-popup.c 12 Nov 2003 21:12:59 -0000 1.7
> +++ em-popup.c 3 Dec 2003 09:54:46 -0000
> @@ -21,7 +21,9 @@
> 
>  #include <camel/camel-folder.h>
>  #include <camel/camel-mime-message.h>
> +#include <camel/camel-medium.h>
>  #include <camel/camel-string-utils.h>
> +#include <camel/camel-url.h>
> 
>  static void emp_standard_menu_factory(EMPopup *emp, EMPopupTarget *target, void *data);
> 
> @@ -500,10 +502,14 @@
>      || g_ascii_strncasecmp(uri, "https:", 6) == 0)
>  mask &= ~EM_POPUP_URI_HTTP;
>  if (g_ascii_strncasecmp(uri, "mailto:", 7) == 0)
> - mask &= ~EM_POPUP_URI_MAILTO;
> + if (g_strrstr (uri, "List-Post") != NULL) {
> + mask &= ~EM_POPUP_URI_LIST;
> + } else {
> + mask &= ~EM_POPUP_URI_MAILTO;
> + }
>  else
>  mask &= ~EM_POPUP_URI_NOT_MAILTO;
> -
> +
>  t->mask = mask;
> 
>  return t;
> @@ -650,7 +656,13 @@
>  { EM_POPUP_ITEM, "10.part.03", N_("Reply to _All"), G_CALLBACK(emp_part_popup_reply_all), NULL, "reply_to_all.xpm", EM_POPUP_PART_MESSAGE},
>  { EM_POPUP_BAR, "20.part", NULL, NULL, NULL, NULL, EM_POPUP_PART_MESSAGE },
>  { EM_POPUP_ITEM, "20.part.00", N_("_Forward"), G_CALLBACK(emp_part_popup_forward), NULL, "forward.xpm", EM_POPUP_PART_MESSAGE },
> -
> +#if 0
> + { EM_POPUP_ITEM, "30.part.00", N_("Request list _help"), G_CALLBACK(emp_list_uri_popup_help), NULL, "faq-16.png", EM_POPUP_PART_IMAGE_MLIST },
> + { EM_POPUP_ITEM, "30.part.01", N_("_Subscribe to list"), G_CALLBACK(emp_list_uri_popup_subscribe), NULL, NULL, EM_POPUP_PART_IMAGE_MLIST },
> + { EM_POPUP_ITEM, "30.part.02", N_("_Unsubscribe from list"), G_CALLBACK(emp_list_uri_popup_unsubscribe), NULL, NULL, EM_POPUP_PART_IMAGE_MLIST },
> + { EM_POPUP_ITEM, "30.part.03", N_("_Post a message to list"), G_CALLBACK(emp_list_uri_popup_post), NULL, NULL, EM_POPUP_PART_IMAGE_MLIST },
> + { EM_POPUP_ITEM, "30.part.04", N_("View list _archives"), G_CALLBACK(emp_list_uri_popup_archives), NULL, NULL, EM_POPUP_PART_IMAGE_MLIST },
> +#endif

Remember to remove this.

>  };
> 
>  static const EMPopupItem emp_standard_part_apps_bar = { EM_POPUP_BAR, "99.object" };
> @@ -681,12 +693,71 @@
>  printf("UNIMPLEMENTED: Add address '%s'\n", t->data.uri);
>  }
> 
> +
> +static void
> +emp_load_url_from_header (EMPopupTarget *t, char *header)
> +{
> + char *url;
> + CamelURL *curl;
> +
> + curl = camel_url_new (t->data.uri, NULL);
> + if (curl == NULL) {
> + printf("Can't get URL: %s\n", t->data.uri);
> + return;
> + }
> + url = "" *)camel_url_get_param(curl, header);
> + camel_object_unref (curl);
> + if (url == NULL)
> + return;
> +
> + if (!strncmp (url, "mailto:", 7)) {
> + em_utils_compose_new_message_with_mailto (url);
> + } else {
> + gnome_url_show (url, NULL);
> + }
> + g_free(url);

^^ you might need to move some of the logic here, or otherwise store
more info in the url param, i.e. when you multiple entries.

> +}
> +static void
> +emp_list_uri_popup_help (GtkWidget *emmli, EMPopupTarget *t)
> +{
> + emp_load_url_from_header (t, "List-Help");
> +}
> +
> +static void
> +emp_list_uri_popup_subscribe (GtkWidget *emmli, EMPopupTarget *t)
> +{
> + emp_load_url_from_header (t, "List-Subscribe");
> +}
> +
> +static void
> +emp_list_uri_popup_unsubscribe (GtkWidget *emmli, EMPopupTarget *t)
> +{
> + emp_load_url_from_header (t, "List-Unsubscribe");
> +}
> +
> +static void
> +emp_list_uri_popup_post (GtkWidget *emmli, EMPopupTarget *t)
> +{
> + emp_load_url_from_header (t, "List-Post");
> +}
> +
> +static void
> +emp_list_uri_popup_archives (GtkWidget *emmli, EMPopupTarget *t)
> +{
> + emp_load_url_from_header (t, "List-Archive");
> +}
> +
> +
>  static EMPopupItem emp_standard_uri_popups[] = {
>  { EM_POPUP_ITEM, "00.uri.00", N_("_Open Link in Browser"), G_CALLBACK(emp_uri_popup_link_open), NULL, NULL, EM_POPUP_URI_NOT_MAILTO },
>  { EM_POPUP_ITEM, "00.uri.10", N_("Se_nd message to..."), G_CALLBACK(emp_uri_popup_address_send), NULL, NULL, EM_POPUP_URI_MAILTO },
>  { EM_POPUP_ITEM, "00.uri.20", N_("_Add to Addressbook"), G_CALLBACK(emp_uri_popup_address_add), NULL, NULL, EM_POPUP_URI_MAILTO },
> + { EM_POPUP_ITEM, "00.uri.30", N_("Request list _help"), G_CALLBACK(emp_list_uri_popup_help), NULL, "faq-16.png", EM_POPUP_URI_LIST },
> + { EM_POPUP_ITEM, "00.uri.40", N_("_Subscribe to list"), G_CALLBACK(emp_list_uri_popup_subscribe), NULL, NULL, EM_POPUP_URI_LIST },
> + { EM_POPUP_ITEM, "00.uri.50", N_("_Unsubscribe from list"), G_CALLBACK(emp_list_uri_popup_unsubscribe), NULL, NULL, EM_POPUP_URI_LIST },
> + { EM_POPUP_ITEM, "00.uri.60", N_("_Post a message to list"), G_CALLBACK(emp_list_uri_popup_post), NULL, NULL, EM_POPUP_URI_LIST },
> + { EM_POPUP_ITEM, "00.uri.70", N_("View list _archives"), G_CALLBACK(emp_list_uri_popup_archives), NULL, NULL, EM_POPUP_URI_LIST },
>  };
> -
>  /* ********************************************************************** */
> 
>  #define LEN(x) (sizeof(x)/sizeof(x[0]))
> @@ -755,6 +826,7 @@
>  break;
>  case EM_POPUP_TARGET_PART: {
>  GList *apps = gnome_vfs_mime_get_short_list_applications(target->data.part.mime_type);
> + gboolean is_mlist_icon = FALSE;
> 
>  /* FIXME: use the snoop_part stuff from em-format.c */
>  if (apps == NULL && strcmp(target->data.part.mime_type, "application/octet-stream") == 0) {
> @@ -762,15 +834,13 @@
> 
>  filename = camel_mime_part_get_filename(target->data.part.part);
> 
> - if (filename) {
> - /* GNOME-VFS will misidentify TNEF attachments as MPEG */
> - if (!strcmp (filename, "winmail.dat"))
> - name_type = "application/vnd.ms-tnef";
> - else
> - name_type = gnome_vfs_mime_type_from_name(filename);
> - if (name_type)
> - apps = gnome_vfs_mime_get_short_list_applications(name_type);
> - }
> + /* GNOME-VFS will misidentify TNEF attachments as MPEG */
> + if (!strcmp (filename, "winmail.dat"))
> + name_type = "application/vnd.ms-tnef";
> + else
> + name_type = gnome_vfs_mime_type_from_name(filename);
> + if (name_type)
> + apps = gnome_vfs_mime_get_short_list_applications(name_type);

again, don't change this logic, filename could be unset -> instant crash.
>  }
> 
>  if (apps) {
> Index: em-popup.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-popup.h,v
> retrieving revision 1.4
> diff -u -r1.4 em-popup.h
> --- em-popup.h 12 Nov 2003 21:12:59 -0000 1.4
> +++ em-popup.h 3 Dec 2003 09:54:46 -0000
> @@ -98,12 +98,14 @@
>  EM_POPUP_URI_HTTP = 1<<0,
>  EM_POPUP_URI_MAILTO = 1<<1,
>  EM_POPUP_URI_NOT_MAILTO = 1<<2,
> + EM_POPUP_URI_LIST = 1<<3,
>  };
> 
>  /* Flags that describe TARGET_PART */
>  enum {
>  EM_POPUP_PART_MESSAGE = 1<<0,
>  EM_POPUP_PART_IMAGE = 1<<1,
> + EM_POPUP_PART_IMAGE_MLIST = 1<<2,
>  };
> 
>  /* Flags that describe TARGET_FOLDER */
> Index: em-utils.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-utils.c,v
> retrieving revision 1.8
> diff -u -r1.8 em-utils.c
> --- em-utils.c 19 Nov 2003 21:22:12 -0000 1.8
> +++ em-utils.c 3 Dec 2003 09:54:51 -0000
> @@ -2286,3 +2286,25 @@
>  /* Now empty the local trash folder */
>  mail_empty_trash (NULL, NULL, NULL);
>  }
> +
> +char
> +*em_utils_extract_url (char *url)

I don't really think this needs to go into em-utils, its very
specific, and it isn't really anything to do with extracting a general
url.  It could just be part of the param extracting code.

> +{
> + char *new_url, *head, *tail;
> +
> + head = url;
> + while ((*head != '<') && (*head != NULL))
> + head++;
> + if (*head == 0)
> + return NULL;
> +
> + tail = ++head;
> + while ((*tail != '>') && (*tail != NULL))
> + tail++;
> + if (*tail == 0)
> + return NULL;
> +
> + new_url = g_strndup (head, tail-head);
> +
> + return new_url;
> +}
> Index: em-utils.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-utils.h,v
> retrieving revision 1.4
> diff -u -r1.4 em-utils.h
> --- em-utils.h 27 Oct 2003 21:31:19 -0000 1.4
> +++ em-utils.h 3 Dec 2003 09:54:51 -0000
> @@ -37,6 +37,7 @@
>  struct _CamelStream;
>  struct _CamelMimeMessage;
>  struct _CamelMimePart;
> +struct _CamelMedium;
>  struct _GtkSelectionData;
>  struct _GtkAdjustment;
>  struct _EMsgComposer;
> @@ -118,6 +119,9 @@
>  void em_utils_expunge_folder (struct _GtkWidget *parent, struct _CamelFolder *folder);
>  void em_utils_empty_trash (struct _GtkWidget *parent);
> 
> +void em_utils_parse_url_from_medium (struct _CamelMedium *msg, char *header);

I think you left this there by mistake?

> +char *em_utils_extract_url (char *url);
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
>
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]