[UIFe] [FFe] UOA integration needs to support multiple accounts

For improving discoverability, I plan to have an "Add more accounts..." element as the last one in the services selector, which will open the Online Accounts panel.

Calum, unless you propose something else, I'll go for this implementation.

Alternatively, Shotwell could maintain it's list of compatible accounts as is, but for accounts that are not activated display this https://docs.google.com/a/canonical.com/document/d/1UwAQTXgEyZSD3di6fAUS0W18rKxh8TXb1TwsmkgbGG0/edit#heading=h.9dy9njhiq1qo

Sorry, I actually had this written here : https://docs.google.com/a/canonical.com/document/d/1UwAQTXgEyZSD3di6fAUS0W18rKxh8TXb1TwsmkgbGG0/edit#bookmark=kix.lzetjp1y4jv1

About the login/logout buttons, I wouldn't remove them: without them, the session will start as soon as the value of the service selector changes, blocking the user from selecting another service.

Indeed it would make sense to remove them, but only if the overall publishing flow is changed (for instance, making the service selector a dialog of its own) -- but unfortunately I think we don't have time to change all that now.

Alberto, I don't understand. Today, I can select a service and log in, but that doesn't block me from selecting another service. Couldn't we remove the buttons and simply log in automatically whenever the user selects any service? Why would the service selector need to be a dialog in this case?

Hi Adam, and sorry for the late reply. I didn't explain the issue properly, so I'll try again. :-)
The current situation is:

1) The user chooses a picture and chooses the "Publish" option from the menu
2) A dialog appears, showing a combobox with the available services, and a login button
3) (optional) The user chooses the desired service, if different from the already selected one
4) The user clicks on the login button
5) (if the OAuth token has expired) A modal dialog (created in the signon-ui process) pops up, embedding the website login page for OAuth
6) Account informations are retrieved
7) The publishing dialog is ready: the user can change some options, if desired, and start the upload.

Now, point 6 takes some time to execute, and point 5 is even heavier, because besides taking some time, it requires user interaction.
If we remove the login step, and attempt to login automatically, points 5 and 6 will be executed automatically as the user starts publishing, and whenever he changes the publishing service. Now, suppose that the pre-selected service is not what the user wants. There's the risk that the OAuth dialog from step 5 would appear, prompting the user to login to a service which he doesn't want to use. Sure, the user can cancel it and continue selecting another service, but it seems better to me to avoid starting logging in to a service which the user might not want to use.

Alberto, I find that it is uncommon that the Oauth token has expired. I have never been prompted to login on the website before publishing, so clicking the "login" button seems pointless.

If the OAuth web site is truly a pop-up, I can see the problem with automatically launching it, since it would get in the way and steal focus. I think this could be fixed by either

- embedding the web site in the form, so that it doesn't steal focus or block the UI. That way, you can start loading the web site as soon as the the user selects "publish".

- detect whether the OAuth token has not expired, and skip the login step.

Hi Aaron, thanks for your advice!
So, the embedding can be done, in theory, but I wouldn't recommend it for a series of reasons of which the main one is the dialog sizing. But your suggestion of checking whether the OAuth token is still valid before showing the login button is a good one.

Indeed it makes the code a bit more complicated, but that's life. :-)

Patch is ready for review (I hope I submitted it on the correct project):

https://code.launchpad.net/~mardy/shotwell/uoa-update/+merge/124126

Here are the links to the posts to the translation and documentation teams:
https://lists.ubuntu.com/archives/ubuntu-translators/2012-September/005588.html
https://lists.ubuntu.com/archives/ubuntu-doc/2012-September/016856.html

Also, the link to the MR given in the previous comment is wrong; here's the correct one:
https://code.launchpad.net/~mardy/shotwell/uoa-update/+merge/124659

+1 from the Docs Team. Those sound like bug fixes to me.

By the way, I tried Shotwell today, looking for the Publish to Picasa button to see what the current UI looked like, but I couldn't find it. I have Google set up in Ubuntu Online Accounts.

Jeremy, I had that problem because my Google account was set up as a Jabber account. It has to be set up as a Google account to work.

Ah, I found it. File>Publish. I also removed my Google account and re-added it but maybe that wasn't needed.

Have you done any testing with this modified patch? Are the new changes doing what they are supposed to be doing?

Shotwell fails to build today (with or without your patch): http://paste.ubuntu.com/1211325/

The build failure is known upstream:

http://redmine.yorba.org/issues/5803

We'll fix this very soon.

The patch 06_uoa.patch is slightly out of date and won't cleanly apply to Shotwell in git master. Alberto, could you bring it up to date?

Never mind - I realized the patch needs to apply to 0.12.90 since the changes in git master are as yet unreleased.

OK - I applied the current patch to the 0.12.90 tarball and built. (I had to use Vala 0.17.6.) I noticed a few things:

- In the Online Accounts panel, the list of accounts that integrate with Shotwell includes Facebook and Google but not Yahoo!/Flickr. Is Flickr support on the way? If so, will it be tracked in this bug or is there a different bug for it?

- When I choose my Facebook account in the publishing pane, I see a slew of critical errors in the terminal where I've run Shotwell:

(shotwell:2089): GLib-GIO-CRITICAL **: g_dbus_gvalue_to_gvariant: assertion `type != NULL' failed
(shotwell:2089): GLib-CRITICAL **: g_variant_new_variant: assertion `value != NULL' failed
(shotwell:2089): GLib-CRITICAL **: g_variant_get_type: assertion `value != NULL' failed
(shotwell:2089): GLib-CRITICAL **: g_variant_type_is_subtype_of: assertion `g_variant_type_check (type)' failed
(shotwell:2089): GLib-CRITICAL **: g_variant_builder_add_value: assertion `!GVSB(builder)->expected_type || g_variant_is_of_type (value, GVSB(builder)->expected_type)' failed
(shotwell:2089): GLib-CRITICAL **: g_variant_builder_end: assertion `GVSB(builder)->offset >= GVSB(builder)->min_items' failed

Publishing does seem to work, however.

- In Makefile.plugins.mk, I noticed that you've added new definitions for EXT_PKGS/PKGS which replace the old ones. Did you mean to use += to append to the old definitions? (If not, your patch should remove the old definitions).

I'll emphasize that I haven't read the code - these are just points I noticed when trying to build and run.

+1 from the translations team.

One question, though: are such strings presented to the user?

host.post_error(new Spit.Publishing.PublishingError.SERVICE_ERROR("Authentication failed"));

If so, they should be marked for translations.

@David: these strings are not shown to the user, just used in debug messages.

@Adam:
  - flickr is supported indeed, but the control panel was not showing it because of https://bugs.launchpad.net/online-accounts-gnome-control-center/+bug/1045866
  - I've seen those warning too, but I assumed that they were not coming from my code, as I'm not using GVariant -- but if they are not there on the upstream, I'll give it a closer look.
  - Yes, what I really wanted is just "+="; the same trick of defining the variable in function of its previous value was already used in the definition of PKGS in that file, so I thought that that was the preferred way.

@Alberto: the critical assertions do not occur with vanilla upstream Shotwell. We don't like assertions like these and have always been pretty strict about noticing and fixing them in our code.

Al 17/09/12 15:00, En/na Alberto Mardegan ha escrit:
> Hi all!
> I'm requesting a freeze exception to land a couple of improvements to
> the integration of Shotwell with Online Accounts:
>
> (the context is uploading an image from Shotwell to some online service
> supported by Online Accounts)
>
> * The service selector should display multiple entries for the service
> with the username associated with the account in the selector.
>
> * As last option in the service selection we add "Add new account...",
> which opens the Online Account applet.
>
> * Login/Logout button in the publishing dialog isn't necessary anymore,
> it's now removed.
>
> Link:
> https://bugs.launchpad.net/ubuntu/+source/shotwell/+bug/1046461
>
> Ciao,
> Alberto
>

"Multiple account support is broken, if a user has more than one account
for facebook it just uses the first account returned from UOA."

This sounds like a serious bug that needs fixing. +1 from translators.

Cheers,
David.

--
David Planella
Ubuntu Translations Coordinator
www.ubuntu.com / www.davidplanella.wordpress.com
www.identi.ca/dplanella / www.twitter.com/dplanella

When can a fix for this be ready?

I updated the merge request, also adding 5 new translatable strings (4 are from the shotwell.application file, the other one is "Add more accounts...").

@Adam: you are absolutely right. I hunted down the critical messages, and they were due to a missing cast (without it Vala was trying to send an enumerated type to D-Bus, instead of just an integer). This should now be fixed.
Unfortunately I had to remove the "--fatal-warnings" compilation flag, because Gtk.Table has just been deprecated and it's causing lots of warnings.

I tested the updated merge proposal with a Flickr and Google (Picasa) account, and both worked fine for me.

Pasting a line by Iulian from the release team (from IRC):

12:16 < iulian> mardy: OK briliant. It looks good. You can go ahead and upload but it won't get in until beta 2 is released.

Alberto: we just released Shotwell 0.13.0, so you'll now need to merge your changes with that release. In 0.13.0 we kept --fatal-warnings, but turned on --enable-deprecated to work around the same Gtk.Table problem.

This bug was fixed in the package shotwell - 0.13.0-0ubuntu2

---------------
shotwell (0.13.0-0ubuntu2) quantal-proposed; urgency=low

  [ Alberto Mardegan ]
  * debian/patches/06_uoa.patch (LP: #1046461)
    - Support multiple accounts per service
    - Attempt automatic login
    - Remove logout buttons
 -- Ken VanDine <email address hidden> Mon, 24 Sep 2012 11:10:48 -0400