After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 794012 - Re-add "add" command-line option
Re-add "add" command-line option
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Online Accounts
unspecified
Other All
: Normal normal
: ---
Assigned To: GNOME Online Accounts maintainer(s)
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-03-03 11:55 UTC by Bastien Nocera
Modified: 2018-03-19 10:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Revert "Revert "online-accounts: Accept requests for account creation on ..."" removing the possibility to preseed data. (2.87 KB, patch)
2018-03-05 08:54 UTC, Andrea Azzarone
committed Details | Review
online-accounts: Restore the "add" command (2.56 KB, patch)
2018-03-05 17:03 UTC, Debarshi Ray
committed Details | Review

Description Bastien Nocera 2018-03-03 11:55:26 UTC
As per:
https://bugzilla.gnome.org/show_bug.cgi?id=789469#c9

This feature is used for applications to request Online Accounts to be setup, not just from web browsers.

+++ This bug was initially created as a clone of Bug #789469 +++

In bug 694313, we enabled browsers to trigger the addition of a new account and passing the cookies over D-Bus to avoid asking for the credentials a second time. The idea was to prompt the user to add her Facebook account in GNOME when the browser detects a log-in. However, it never went beyond the proof-of-concept stage. It's been more than four years and as far as I know, nobody uses this feature.

The extra cookie management that's necessary to implement this feature has now exposed us to a WebKitGTK+ bug (https://bugs.webkit.org/show_bug.cgi?id=175265) that hasn't been fixed in a while. It is preventing users from adding multiple accounts backed by the same provider (bug 781005), which can be a pretty important thing.

I propose that we drop this feature. We have waited long enough for a proper fix in WebKitGTK+, and the proposed workarounds are pretty abhorrable. It doesn't make sense to endure all this for code that nobody uses.
Comment 1 Andrea Azzarone 2018-03-05 08:54:30 UTC
Created attachment 369319 [details] [review]
Revert "Revert "online-accounts: Accept requests for account creation on ..."" removing the possibility to preseed data.

This reverts commit 5a04e40fefb350c2f7df2e9f1eed6bce373f5cce.
Comment 2 Debarshi Ray 2018-03-05 09:21:01 UTC
Review of attachment 369319 [details] [review]:

Thanks for the patch. It looks sane, but I haven't actually tested it.

Some nitpicks about the commit message: 
(a) It only reverts parts of commit 5a04e40fefb350c2 that don't use the preseed API. It will be good to clarify that, since that seemingly minor distinction has led to this bug report. :)
(b) It will be nicer to have a more readable summary that fits within 72 characters, etc. instead of a long auto-generated double-revert string.
Comment 3 Debarshi Ray 2018-03-05 17:02:25 UTC
Review of attachment 369319 [details] [review]:

::: panels/online-accounts/cc-online-accounts-panel.c
@@ +1,3 @@
 /* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- */
 /*
+ * Copyright (C) 2011, 2017 Red Hat, Inc.

Another nitpick: spurious change.

@@ +320,3 @@
+        g_variant_get_child (parameters, 1, "v", &v);
+        if (g_variant_is_of_type (v, G_VARIANT_TYPE_STRING))
+          provider_name = g_variant_get_string (v, NULL);

Now that I look at it, this code makes me nervous. "provider_name" is tied to the lifetime of "v", but we are using "provider_name" after unreffing "v". This can only work if "v" was directly using the memory owned by "parameters", which I am not sure is the case here.

Anyway, this code has been working for years, so let's leave it like this for the 3.28.0 release. We can fix it afterwards.
Comment 4 Debarshi Ray 2018-03-05 17:03:39 UTC
Created attachment 369357 [details] [review]
online-accounts: Restore the "add" command

I took the liberty to fix up the nitpicks and pushed it. Thanks for the patch and testing the Online Accounts panel, Andrea!
Comment 5 Michael Catanzaro 2018-03-19 02:05:02 UTC
(In reply to Bastien Nocera from comment #0)
> The extra cookie management that's necessary to implement this feature has
> now exposed us to a WebKitGTK+ bug
> (https://bugs.webkit.org/show_bug.cgi?id=175265) that hasn't been fixed in a
> while. It is preventing users from adding multiple accounts backed by the
> same provider (bug 781005), which can be a pretty important thing.
> 
> I propose that we drop this feature. We have waited long enough for a proper
> fix in WebKitGTK+, and the proposed workarounds are pretty abhorrable. It
> doesn't make sense to endure all this for code that nobody uses.

That WebKit bug was fixed four months ago. Are you aware of any remaining problem in WebKit?
Comment 6 Debarshi Ray 2018-03-19 10:23:34 UTC
(In reply to Michael Catanzaro from comment #5)
> That WebKit bug was fixed four months ago. Are you aware of any remaining
> problem in WebKit?

No, I am not aware of any problem in WebKit today. However, we don't have the code anymore.

To summarize, we were only messing around with cookies to let web browsers pass their cookies to the embedded WebView in the Online Accounts panel to skip the authentication step while adding an account if the user was already logged in through the web browser. ie. the feature in bug 694313. Since none of these web browser extensions ever progressed beyond the prototype stage, it doesn't make sense to carry the enabling code in gnome-online-accounts either.