Re: [Epiphany] mega-patch

[Marco Pesenti Gritti <mpgritti oltrelinux com>]

On Fri, 2003-02-14 at 21:39, Marco Pesenti Gritti wrote:
> On Fri, 2003-02-14 at 20:12, James Willcox wrote:
> > On Fri, 2003-02-14 at 07:29, Marco Pesenti Gritti wrote:
> > > On Fri, 2003-02-14 at 05:42, James Willcox wrote:
> > > > Hi Marco,
> > > > 
> > > > Here's a patch that does 2 things:  adds load feedback to tabs, and adds
> > > > a context menu to the bookmark editor. 
> > > > http://www.cs.indiana.edu/~jwillcox/epiphany-loading-and-menu.png
> > > > 
> > > > It really should be 2 separate patches, but I am lame and didn't want to
> > > > separate it.  Of course, if you want one and not the other I will go
> > > > ahead and do it. :)
> > > > 
> > > > Thanks,
> > > > James
> > > > ----
> > > > 
> > > 
> > > Cool :) Review follow.
> > > 
> > 
> > Ah, all very good points.  Thanks.  I fixed them all except the
> > following and committed it....attached is the patch
> 
> Thanks.
> 
> > > > +	/* setup load feedback image */
> > > > +	loading_pixbuf = gdk_pixbuf_animation_new_from_file (ephy_file ("epiphany-tab-loading.gif"), NULL);
> > > > +	loading_image = gtk_image_new_from_animation (loading_pixbuf);
> > > > +	g_object_unref (loading_pixbuf);
> > > > +	gtk_box_pack_start (GTK_BOX (hbox), loading_image, FALSE, FALSE, 0);
> > > > +
> > > 
> > > I'd prefer if we register this as a stock icon. Have a look at
> > > lib/ephy-stock-icons.c. So it's themable ...
> > 
> > I wanted to do this too, but currently gtk+ doesn't support registering
> > GdkPixbufAnimation stuff as stock icons.  I was thinking about using the
> > gnome icon theme stuff to do it, but it doesn't support gif (or mng).
> 
> Oh, ok. Not a big deal, we can fix it later, when it's possible to do
> it.
> 
> > Also, I totally ripped that gif out of mozilla....there may be licensing
> > issues or something.  One thing I was thinking about, is that we could
> > snarf the image by grabbing it through the mozilla vfs.  The tab loading
> > icon is available through chrome://communicator/skin/icons/loading.gif. 
> > That would also allow the icon to be themable (through the mozilla
> > chrome system).  What do you think?
> 
> I think the best would be to use another icon. What about this ?
> http://jimmac.musichall.cz/ikony/i40/stock_refresh-blue.png
> Rotate it and make an animated gif should not be that hard.
> I'm really not good with gimp but I guess we can try to get that done in
> some way if you like the idea. (Btw we could just commit the patch with
> this static icon, to not risk conflicts).
> 

Ouch. Just noticed the ... on all context menus items. I dont think they
are necessary, any particular reason to put them ?

Thanks,
Marco