Re: [evolution-patches] Please review my patch for bug #47131

[Antonio Xu <antonio xu sun com>]

Hello Jeffrey Stedfast

Thanks for you review my patch, I have created a new patch according to 
your advice, Could you review it?  By the way, My patch need to be 
committed into both trunk and branch.

Regards,
Anto
Jeffrey Stedfast wrote:

> Antonio Xu wrote:
>
> > Hello All,
> >                Could you review my patch for bug #47131?
> >                Bug Summary:Drag link from mozilla browser and drop it 
> > into To field of evolution composer
> >                I add the drop type of mozilla link to make composer 
> > support to handle mozilla's Dnd Data, I found the same code was used 
> > in gtkhtml, So I think it is appropriate to add mozilla link support 
> > in mail composer
> >
> > Thanks
> > Antonio Xu
> >
> >
> > ------------------------------------------------------------------------
> >
> > Index: composer/e-msg-composer.c
> > ===================================================================
> > RCS file: /cvs/gnome/evolution/composer/e-msg-composer.c,v
> > retrieving revision 1.395.2.4
> > diff -u -r1.395.2.4 e-msg-composer.c
> > --- composer/e-msg-composer.c    23 Jul 2003 18:17:20 -0000    1.395.2.4
> > +++ composer/e-msg-composer.c    30 Jul 2003 16:47:17 -0000
> > @@ -121,12 +121,14 @@
> > enum {
> >     DND_TYPE_MESSAGE_RFC822,
> >     DND_TYPE_TEXT_URI_LIST,
> > +    DND_TYPE__NETSCAPE_URL,
>
> I think I'd prefer DND_TYPE_NETSCAPE_URL rather than the double 
> underscore.
>
> >     DND_TYPE_TEXT_VCARD,
> > };
> >
> > static GtkTargetEntry drop_types[] = {
> >     { "message/rfc822", 0, DND_TYPE_MESSAGE_RFC822 },
> >     { "text/uri-list", 0, DND_TYPE_TEXT_URI_LIST },
> > +    { "_NETSCAPE_URL", 0, DND_TYPE__NETSCAPE_URL },
> >     { "text/x-vcard", 0, DND_TYPE_TEXT_VCARD },
> > };
> >
> > @@ -2568,6 +2570,7 @@
> >         camel_object_unref (stream);
> >         break;
> >     case DND_TYPE_TEXT_URI_LIST:
> > +    case DND_TYPE__NETSCAPE_URL:
> >         d(printf ("dropping a text/uri-list\n"));
> >         tmp = g_strndup (selection->data, selection->length);
> >         urls = g_strsplit (tmp, "\n", 0);
> > @@ -2585,16 +2588,18 @@
> >
> >                 if (url == NULL)
> >                     continue;
> > +
> > +                if (strcasecmp(url->protocol, "http")) {
>
> I prefer a space before the '('
>
> also... it'd be better to say:
>
> if (strcasecmp (url->protocol, "http") != 0) {
> ...
>
> it makes it clearer that you mean "protocol is NOT http"
>
> which brings us to another issue: what about ftp/https/etc urls?
>
> since really what you seem to mean is "protocol IS file", you should 
> probably do:
>
> if (!strcasecmp (url->protocol, "file")) {
> ...
>
> right? or am I misunderstanding?
>
> > +                    filename = url->path;
> > +
> > +                    e_msg_composer_attachment_bar_attach
> > +                        (E_MSG_COMPOSER_ATTACHMENT_BAR 
> > (composer->attachment_bar),
> > +                         filename);
> >                
> > -                filename = url->path;
> > +                    g_free (filename);
> > +                }
> >                 url->path = NULL;
> >                 camel_url_free (url);
> > -               
> > -                e_msg_composer_attachment_bar_attach
> > -                    (E_MSG_COMPOSER_ATTACHMENT_BAR 
> > (composer->attachment_bar),
> > -                     filename);
> > -               
> > -                g_free (filename);
> >             }
> >         }
> >        
> > Index: composer/ChangeLog
> > ===================================================================
> > RCS file: /cvs/gnome/evolution/composer/ChangeLog,v
> > retrieving revision 1.544.2.8
> > diff -u -r1.544.2.8 ChangeLog
> > --- composer/ChangeLog    29 Jul 2003 15:38:08 -0000    1.544.2.8
> > +++ composer/ChangeLog    30 Jul 2003 16:47:26 -0000
> > @@ -1,3 +1,9 @@
> > +2003-07-31  Antonio Xu  <antonio xu sun com>
> > +
> > +    * e-msg-composer.c (drag_data_received): add the drop type of
> > +    mozilla browser link to make composer support to handle mozilla
> > +    DnD data.  [#47131]
> > +
> > 2003-07-28  Antonio Xu <antonio xu sun com>
> >
> >        * e-msg-composer-attachment-bar.c  (update): add
> >  


Index: composer/ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/composer/ChangeLog,v
retrieving revision 1.544.2.8
diff -u -r1.544.2.8 ChangeLog
--- composer/ChangeLog	29 Jul 2003 15:38:08 -0000	1.544.2.8
+++ composer/ChangeLog	31 Jul 2003 04:27:31 -0000
@@ -1,3 +1,9 @@
+2003-07-31  Antonio Xu  <antonio xu sun com>
+
+	* e-msg-composer.c (drag_data_received): add the drop type of
+	mozilla browser link to make composer support to handle mozilla
+	DnD data.  [#47131]
+
 2003-07-28  Antonio Xu <antonio xu sun com>
 
        * e-msg-composer-attachment-bar.c  (update): add
Index: composer/e-msg-composer.c
===================================================================
RCS file: /cvs/gnome/evolution/composer/e-msg-composer.c,v
retrieving revision 1.395.2.4
diff -u -r1.395.2.4 e-msg-composer.c
--- composer/e-msg-composer.c	23 Jul 2003 18:17:20 -0000	1.395.2.4
+++ composer/e-msg-composer.c	31 Jul 2003 04:28:00 -0000
@@ -121,12 +121,14 @@
 enum {
 	DND_TYPE_MESSAGE_RFC822,
 	DND_TYPE_TEXT_URI_LIST,
+	DND_TYPE_NETSCAPE_URL,
 	DND_TYPE_TEXT_VCARD,
 };
 
 static GtkTargetEntry drop_types[] = {
 	{ "message/rfc822", 0, DND_TYPE_MESSAGE_RFC822 },
 	{ "text/uri-list", 0, DND_TYPE_TEXT_URI_LIST },
+	{ "_NETSCAPE_URL", 0, DND_TYPE_NETSCAPE_URL },
 	{ "text/x-vcard", 0, DND_TYPE_TEXT_VCARD },
 };
 
@@ -2568,6 +2570,7 @@
 		camel_object_unref (stream);
 		break;
 	case DND_TYPE_TEXT_URI_LIST:
+	case DND_TYPE_NETSCAPE_URL:
 		d(printf ("dropping a text/uri-list\n"));
 		tmp = g_strndup (selection->data, selection->length);
 		urls = g_strsplit (tmp, "\n", 0);
@@ -2585,16 +2588,18 @@
 
 				if (url == NULL)
 					continue;
-				
-				filename = url->path;
+
+				if (!strcasecmp (url->protocol, "file")) {
+					filename = url->path;
+
+					e_msg_composer_attachment_bar_attach
+						(E_MSG_COMPOSER_ATTACHMENT_BAR (composer->attachment_bar),
+					 	filename);
+
+					g_free (filename);
+				}
 				url->path = NULL;
 				camel_url_free (url);
-				
-				e_msg_composer_attachment_bar_attach
-					(E_MSG_COMPOSER_ATTACHMENT_BAR (composer->attachment_bar),
-					 filename);
-				
-				g_free (filename);
 			}
 		}