GNOME Bugzilla – Bug 600190
Postr? Postasa! Uploader for PicasaWeb
Last modified: 2019-02-22 03:17:21 UTC
Created attachment 146617 [details] [review] Preliminary patch adding PicasaWeb upload support. Alright, so the name needs a bit of work, and so does the patch, but here's a patch that adds support for authenticating and uploading to PicasaWeb. The patch adds Postasa for uploading to PicasaWeb using Philip Withnall's libgdata's PicasaWeb support. It provides a configuration dialog which lets you login to PicasaWeb, and a menu item to upload, like Postr (which I copied from Postr). If you're not logged in when you try to upload, it pops open the configuration dialogue. After uploading, there's a pop-up (too annoying?) listing the number of files successfully uploaded, and any warning messages that might have been received. All uploaded files go to the Drop Box, which is a default album in PicasaWeb when a user hasn't specified an alternative one. * I'm no autotools/make wizard, so I mostly mimicked what exif-display and postr were doing in there. * EogPostasaPlugin has Private data. I'm not a GObject wizard either, so I fear I might have munged something :) * I am not sure what the proper way to go about making strings localisable is. I think Postr uses N_("string in here"). Is that so? * I don't want the configuration dialogue to send a response and return from gtk_dialog_run () when "Login" is clicked, so I catch it first and stop_emission (). This might be a bad way to do it :) * I'm not sure that the popup at the end of uploads is desirable if they were all successful (though I like feedback when they're done) * gdata_picasaweb_service_upload_file () doesn't have an asynchronous counterpart yet and might block. I will try to add one to libgdata if that's necessary. (Authentication is done asynchronously, though :) * Lucas Rocha is assigned credit, since he was listed for Postr and most of its code was recycled for this. * If there's demand, I could add an album chooser to the configuration dialogue. Yah, so, I'll be glad to incorporate corrections and feedback if there's interest in this plugin.
(In reply to comment #0) > Created an attachment (id=146617) [details] [review] > Preliminary patch adding PicasaWeb upload support. > > Alright, so the name needs a bit of work, and so does the patch, but here's a > patch that adds support for authenticating and uploading to PicasaWeb. Actually I think Postasa is kind a neat as a project name. :) If we leave the .eog-plugin like it is now, where it tells the user directly what he's going to be able to do, there shouldn't be really a problem with it. > > The patch adds Postasa for uploading to PicasaWeb using Philip Withnall's > libgdata's PicasaWeb support. It provides a configuration dialog which lets > you login to PicasaWeb, and a menu item to upload, like Postr (which I copied > from Postr). If you're not logged in when you try to upload, it pops open the > configuration dialogue. After uploading, there's a pop-up (too annoying?) > listing the number of files successfully uploaded, and any warning messages > that might have been received. All uploaded files go to the Drop Box, which is > a default album in PicasaWeb when a user hasn't specified an alternative one. > In case you're worried about the menu icon, you can take a look at the reload plugin that's shipped with eog. It shows a way how to add a menu icon without depending on the few predefined slots eog provides. It's a bit more complex though and involves XML, so it wouldn't be a problem if you don't want to change it. > * I'm no autotools/make wizard, so I mostly mimicked what exif-display and > postr were doing in there. Looks good. Just add $(UI_FILES) to EXTRA_DIST in the Makefile.am or the GtkBuilder file won't be picked up when we do make dist. > * EogPostasaPlugin has Private data. I'm not a GObject wizard either, so I fear > I might have munged something :) Spontaneously I can't spot a problem here. :) > * I am not sure what the proper way to go about making strings localisable is. > I think Postr uses N_("string in here"). Is that so? In this case it is correct as the strings are used in a "static const" construct. Strings wrapped in N_() need to be manually passed through gettext on runtime (GTK does it here). Normally wrapping them in _() is enough, which feeds them to gettext automatically. The GtkBuilder file solves it with an XML attribute (that can be switched in Glade) . > * I don't want the configuration dialogue to send a response and return from > gtk_dialog_run () when "Login" is clicked, so I catch it first and > stop_emission (). This might be a bad way to do it :) Why do you wan't it that way? Anyway, you don't actually need to use gtk_dialog_run to show a dialog. It's usually enough if you use gtk_widget_show_all or gtk_window_present on it. You need to connect to the dialog buttons yourself and refactor some code blocks as the program won't wait for the user clicking a button then (making the dialog somewhat async). > * I'm not sure that the popup at the end of uploads is desirable if they were > all successful (though I like feedback when they're done) > * gdata_picasaweb_service_upload_file () doesn't have an asynchronous > counterpart yet and might block. I will try to add one to libgdata if that's > necessary. (Authentication is done asynchronously, though :) Well, I would propose a progress dialog, although not sure if an async libgdata is necessary for this. That could show an progressbar (even if it's just in activity mode, if there's no way to calculate progress) and messages. It would give the user at least some feedback that something is happening. If an async libgdata is a problem, we could likely explore if it could be incorporated into the EogJobs framework. It will likely block other jobs (like loading) then. No plugin tried that before, so no idea if it's possible. > * Lucas Rocha is assigned credit, since he was listed for Postr and most of its > code was recycled for this. I would take him out of the .eog-plugin file as I think he wouldn't be able to respond to possible support requests coming in over email. I was going to ask for a license comment (in the .c and .h) anyway, so I'd say it's enough to credit him in that ("Based on code by Lucas..."). But you can ask him as well if you like to be on the safe side. :) > * If there's demand, I could add an album chooser to the configuration > dialogue. I guess demand will come sooner or later, but it's not a "must have" necessary for inclusion. Another issue I spotted while compiling: You sometimes use printf-style functions without providing a format string (passing the string that should be printed directly). This is somewhat dangerous, especially when you work with translated strings where you don't really have the control over the passed string. For example that string could for some reason contain a sequence that looks like a printf-token and cause segfaults and other problems. gcc output looks like the following: CC eog-postasa-plugin.lo eog-postasa-plugin.c: In function 'picasaweb_upload_cb': eog-postasa-plugin.c:163: warning: format not a string literal and no format arguments eog-postasa-plugin.c:168: warning: format not a string literal and no format arguments So, always provide a format string for such functions, even if it's just "%s". :)
Ahoy. So, I've implemented most of the commendations, use GAsyncReadyResult and friends to make the upload asynchronous (at least until bug 600262 is resolved) to no longer block the UI while uploading, and added an Uploads window for that shows a list of uploads and their status. I have one problem though: I want to provide Cancel buttons to cancel them, except there's a little issue in libgdata, bug 602057, in that my PicasaWeb upload method in there uses a g_assert () on a condition which is false when I've used a GCancellable to cancel the upload. D'oh. So, I'm wondering whether EoG Plugins is usually compiled with G_DISABLE_ASSERT defined. Right now, I'm removing the cancel buttons if that's not defined with an #ifndef macro :( When it's not defined, is it acceptable to remove the buttons (they'd just lead to crashes anyway) when using a release with the assert (in theory, <0.6), defining something like EOG_POSTASA_DISABLE_CANCEL when doing configure?
Er, I think I misunderstood how G_DISABLE_ASSERT worked, anyway, so I don't think it matters whether EoG Plugins is compiled with it. The question about whether it's acceptable to enable/disable the cancel feature based on Libgdata version still stands, though :)
Er, nevermind that babble. Problem was in some work-in-progress code of mine. Will attach an updated patch after some clean up.
Created attachment 148000 [details] [review] Much improved patch adding PicasaWeb upload support This still doesn't have a menu icon, but I was under the impression that GNOME was moving away from ambiguous icons, and I have no icon making skills. If someone did care for one, I might spend some time trying. So, changes include * asynchronous uploading * upload progress window (has a treeview listing files uploading, pulses for progress, and reports errors) * memory handling is much improved (I had to look at source files sometimes :|). I'm currently not explicitly freeing the GCancellable or GdkPixbuf that go into the GtkListStore for the uploads window, because they exist until EoG exits anyway; if that's still undesirable, I'll be more explicit about that.) * Lucas is now ref'd in the .[ch] files rather than the .eog-plugin file * now requires the not-yet-released libgdata 0.6.0. There was a bug in how a GDataUploadStream was disposed (bug 602156) that meant cancelling uploads could cause a crash (ugh!). If there's interest and it's allowed, I could make the Cancel widgets only available when compiled with libgdata 0.6.0, and just have them hidden for 0.5.0. However, I expect libgdata 0.6.0 will be out before EoG Plugins 2.30, so hopefully this is alright?
(In reply to comment #5) > Created an attachment (id=148000) [details] [review] > Much improved patch adding PicasaWeb upload support > > This still doesn't have a menu icon, but I was under the impression that GNOME > was moving away from ambiguous icons, and I have no icon making skills. If > someone did care for one, I might spend some time trying. > > So, changes include > * asynchronous uploading > * upload progress window (has a treeview listing files uploading, pulses for > progress, and reports errors) Looks neat so far. :) Two (not really critical) things I spotted during my quick test: - When I'm asked for my login, the entries and buttons should be made insensitive while logging in. Also, if the login worked the dialog should be automatically closed. - I should not be able to cancel already completed uploads (not sure if that is possible). Maybe just remove (successfully) completed jobs from the list and close it when it's empty? > * memory handling is much improved (I had to look at source files sometimes > :|). I'm currently not explicitly freeing the GCancellable or GdkPixbuf that > go into the GtkListStore for the uploads window, because they exist until EoG > exits anyway; if that's still undesirable, I'll be more explicit about that.) Yeah, we should finish our API docs. :) Due to GCancellable and GdkPixbuf being GObjects the list model will keep a reference on them as long as they are saved in the model. So, if you clear or destroy the model the references to the objects will be removed and if necessary the objects will be disposed. Regarding the lifetime of the thumbnails. Well, that's an interesting story/problem in eog, I didn't think of myself lately (the close confirm dialog is thus affected by this as well). The only thumbnails that are available are the ones of images, that are currently visible in the collection pane + the currently loaded image. That means if I select the first image in my collection and then the last one for uploading (the first one gets out of sight in the collection pane) the thumb of the first could already be released until your upload dialog is created, and thus will spit out some warnings to the console and the image is displayed without preview in the list. Not sure how to nicely handle that yet though. :( > * Lucas is now ref'd in the .[ch] files rather than the .eog-plugin file > * now requires the not-yet-released libgdata 0.6.0. There was a bug in how a > GDataUploadStream was disposed (bug 602156) that meant cancelling uploads could > cause a crash (ugh!). If there's interest and it's allowed, I could make the > Cancel widgets only available when compiled with libgdata 0.6.0, and just have > them hidden for 0.5.0. However, I expect libgdata 0.6.0 will be out before > EoG Plugins 2.30, so hopefully this is alright? Well, this depends on the time libgdata 0.6 will be released before 2.30. If it is too close, it could prevent thorough testing of Postasa.
Created attachment 150857 [details] [review] PicasaWeb upload patch with some changes This update to the patch * makes the login entries insensitive while logging in, * automatically closes the login dialogue upon successful login, * cleans up some trailing whitespace in the code, and * unref's the list store during finalisation * uses a cartoony default thumbnail in the absence of a proper thumbnail for the uploads window's image upload rows. Regarding cancelling completed uploads, I actually like seeing them in the list, especially when I'm going through a large collection. Would it be sufficient to desensitise the single Cancel button when highlighting completed jobs, and the Cancel All if there are no jobs in progress? I will remove completed ones if you think their presence and button sensitivity changes will confuse users, though. I understand the interest in freeing the thumbnails whenever possible, especially when traversing large directories of photos. Perhaps I could regenerate thumbnails if they don't exist when I add them to the uploads pane, or perhaps thumbnails that are selected could remain referenced until they're both out of view and deselected? I am now just displaying a 32x32 bit landscape thumbnail I just made in the Gimp. If we want to use a default image, perhaps using the current theme's icon for images would be better? Relevant patches for proper async uploading in libgdata will make it for 0.6.0. I'll poke Philip Withnall about when 0.6.0 will be released.
Created attachment 150858 [details] Simple Landscape thumbnail temporarily used for unref'd thumbnails. Here is the cute little landscape that the updated patch is using in place of absent thumbnails.
(In reply to comment #8) > Created an attachment (id=150858) [details] > Simple Landscape thumbnail temporarily used for unref'd thumbnails. > > Here is the cute little landscape that the updated patch is using in place of > absent thumbnails. There's already an icon in gnome-icon-theme we can reuse here, although it's more a burnt landscape. ;) http://git.gnome.org/browse/gnome-icon-theme/plain/gnome/256x256/mimetypes/image-x-generic.png?id=b4b65003e9c0df43c1c86c2cbbd203a1b5fb9836
Well well, works pretty well so far. :) One thing that's slightly irritating is that cancelled uploads still show up in Picasa. Nothing serious. I guess I should get your patch reviewed so we can have a first release in one of the next releases and hopefully get some testing. :)
Ah. Yah, libgdata leaves cancelled uploads on the server and doesn't return a GDataEntry so we can delete it (returning NULL instead). I've filed bug 607620 and will write a patch for it there. I'll change the icon to image-x-generic.png by this weekend, if that arid icon is preferable.
Okay, I put the plugin with some changes (e.g. default thumbnail) into the tree and will ship it with 2.29.90 to get some testing. We can work on improvements/bugs from here on in-tree. I just hope there'll be libgdata-0.6.0 in time. ;) commit 6fb64e0ae158c3a580376044610e4e4312e488fe Author: Richard Schwarting <> Date: Wed Feb 10 19:16:14 2010 +0100 Add Postasa plugin allowing uploads to PicasaWeb This new plugin supports uploading images to PicasaWeb using libgdata. It requires the yet unreleased libgdata-0.6.0, but it should be out in time. See bug #600190. (In reply to comment #7) > Regarding cancelling completed uploads, I actually like seeing them in the > list, especially when I'm going through a large collection. Would it be > sufficient to desensitise the single Cancel button when highlighting completed > jobs, and the Cancel All if there are no jobs in progress? I will remove > completed ones if you think their presence and button sensitivity changes will > confuse users, though. Just, saw this. Yes, this would be good. This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.
I e-mailed Philip and says a release should be out by the weekend, at which point I'll need to propose an external dependency bump for libgdata to 0.6.0.