Cairo Menu doesn't check whether bookmarked directories exist

[Gabor Karsay]

Awn and Awn-extras: 0.4.0 installed from source (tarball)

Steps to reproduce: Create a directory, bookmark it, delete the directory
Result: Cairo Menu shows the bookmark in the Places menu.
Expected result: Like in gnome-panel/nautilus the bookmark shouldn't be shown. If you go in Nautilus to Manage Bookmarks, you see, that the bookmark exists, but it has a different icon. So obviously gnome-panel/nautilus checks whether the bookmarked directory exists and doesn't show it if not.
Also seen in: YAMA, Places applet

[Gabor Karsay (gabor-karsay)]

A check for existence would be nice, but only for local directories (not ftp, shh etc.).

[moonbeam (rcryderman)]

Gabor,

Are you sure this is lda bug? It may be an extras bug.

[Gabor Karsay (gabor-karsay)]

Well, you could argue both. Cairo Menu uses this line in gnome-menu-builder.c:
  bookmarks = desktop_agnostic_vfs_gtk_bookmarks_get_bookmarks (bookmarks_parser);

The problem is, that the parsed ~/.gtk-bookmarks can have directories that don't exist anymore. Neither lda nor Cairo Menu perform such a check. I would argue, that lda should do that check and should not return any bookmarks of non-existing directories, but it could be also done in Cairo Menu.

Places applet also uses lda for this, YAMA seems not to use it.

[Mark Lee (malept)]

I think that the bookmarks parser should return everything, and then the applet can do whatever it wants. In the Bookmark object, there's a file property, which has an exists() method. The applet can use that to determine what to do with the bookmark entry. For example, an applet could decide that it would show a "this bookmark does not exist" icon instead of not showing it at all. Therefore, this is an applet-specific problem, not an lda problem.

[Michal Hruby (mhr3)]

Agreed with Mark, it should be handled by applets.

[Gabor Karsay (gabor-karsay)]

Okay, this is a patch for Cairo Menu to check if the bookmarked directory exists.

[onox (onox)]

First look at the patch suggests it doesn't take remote locations into account, of which its exists() method returns False unless the location has been mounted.

[Gabor Karsay (gabor-karsay)]

Oh it works, because Desktop Agnostic doesn't return a path for remote locations, only an uri, that means if (path) should be false for remote locations. The surrounding code works with this assumption, too. Anyway, here is a patch with a different approach.

[onox (onox)]

@moonbeam: see patch from Gabor

[Gabor Karsay (gabor-karsay)]

Fix committed in rev. 1491.