Feature -> 1.6

Mass reassigning of Epiphany bugs to epiphany-maint@b.g.o

Created attachment 32702 [details] [review]
implementation

Not sure if we should also add print-to-file files to recent...

Created attachment 32703 [details] [review]
real patch

Re comment 3, i think we should also include print-to-file files. Is that
included in "real patch"?

No, it's not included.
Adding print-to-file files means we've got to bring back the print progress
listener code...

...which it sounds like you don't wanna do :)

Anyway, print-to-file isn't too common an action -- i'm happy with the rest really.

Created attachment 35330 [details] [review]
updated patch to epiphany HEAD

Using latest egg-recent, I get a HUGE delay (browser unresponsive) while adding
a new recent item. Delaying this patch until that's fixed.

Christian, do you see this with an empty ~/.recently-used as well?

I think that was the try-to-sniff-mime-of-remote files bug that's fixes long since.

Would be nice if someone could take the patch, apply to HEAD (may need a bit of tweaking, not sure if it still applies cleanly), see if it basically works, preferably before feature freeze :)

Comment on attachment 35330 [details] [review]
updated patch to epiphany HEAD

Two things that don't quite feel right:

>+		if (!mDontAddToRecent)

Maybe it's just me, but I hate parsing double negatives. Wouldn't it be more simple to just use 'if (mAddToRecent)' ?


>+	egg_recent_item_add_group (item, _("Epiphany"));
>+	egg_recent_item_add_group (item, _("Web Browser"));

The spec is vague about the exact semantics of the groups, but I believe the names aren't meant to be shown to the user and thus shouldn't be translated. I believe the use case is rather for an application to be able to pick only its own recent files (to be shown in File/Recent menu or something) - if you use the translation and switch locales, you'd get an odd subset of the files.

(In reply to comment #12)
> (From update of attachment 35330 [details] [review] [edit])
> Two things that don't quite feel right:
> 
> >+		if (!mDontAddToRecent)
> 
> Maybe it's just me, but I hate parsing double negatives. Wouldn't it be more
> simple to just use 'if (mAddToRecent)' ?

I don't remember why I wrote it that way; feel free to change it.

> 
> >+	egg_recent_item_add_group (item, _("Epiphany"));
> >+	egg_recent_item_add_group (item, _("Web Browser"));
> 
> The spec is vague about the exact semantics of the groups, but I believe the
> names aren't meant to be shown to the user and thus shouldn't be translated. I
> believe the use case is rather for an application to be able to pick only its
> own recent files (to be shown in File/Recent menu or something) - if you use
> the translation and switch locales, you'd get an odd subset of the files.

I think you're right.

-> 1.12 due to feature and UI freeze.

This shouldn't be using libegg anymore but the GtkRecent* code in GTK+ 2.10.

Created attachment 93252 [details] [review]
reworked patch to SVN trunk and using GtkRecent

The attached patch is a rework of chpe's which applies to SVN trunk, and uses GtkRecent instead of EggRecent stuff.

+static GtkRecentManager *manager = NUstatic GtkRecentManager *manager = NULL;LL;

I don't think we need to cache this; we can just always use the _get_default() when we need it. And you seem to mix using tabs and spaces for indent?

Otherwise looks fine; have you tested that the file gets added to recent in the right cases (download/saving, but not when opening a file loaded to /tmp ) ?

Created attachment 93324 [details] [review]
reworked patch v2

This should be ok. Sorry for the tabs and spaces mess, it should be a bug of Anjuta with 4-char tabs.
Recent Documents element in the gnome panel menu is not working at all (not a single app manage to file an entry in there...maybe a bug in gnome-applets 2.19.x) on my box, so I have not been able to test its integration, but i made a test case with a GtkRecentInfo and some g_debugs (you can find its traces in my previous patch, as I forgot to remove a part of it) and it worked fine, retrieving files once saved, so it should be ok.

I couldn't get this working, is gnome-menus/panel broken?

You can also check out if it works with the GtkFileChooser (recently used). It works.

Thanks Cosimo!

2007-08-26  Diego Escalante Urrelo  <diegoe@gnome.org>

        * lib/ephy-file-helpers.c:
        * lib/ephy-file-helpers.h:
        * embed/mozilla/MozDownload.h:
        * embed/mozilla/MozDownload.cpp:

        Add saved files to the GtkRecent list, gnome-panel is broken so don't
        expect to see the effects of this working there, you can use it on the
        GtkFileChooser dialog however. Fixes Bug #148401. Original patch by
        Christian Persch, updated by Cosimo Cecchi.

Lucky revision 7300.

Just for reference, now that bug #463907 has been fixed, this works good also in the panel menu.