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 678161 - online accounts: network error handling
online accounts: network error handling
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Online Accounts
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Online Accounts maintainer(s)
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-06-15 12:44 UTC by Matthias Clasen
Modified: 2017-04-27 20:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (242.78 KB, image/png)
2012-11-08 16:01 UTC, Allan Day
  Details
network error handling patch (1.35 KB, patch)
2012-11-29 10:09 UTC, Ebru Akagündüz
needs-work Details | Review
online accounts: network error handling (1.27 KB, patch)
2012-11-29 16:19 UTC, Ebru Akagündüz
needs-work Details | Review
network error handling patch (1004 bytes, patch)
2012-12-01 10:39 UTC, Ebru Akagündüz
reviewed Details | Review
network error handling patch (1.01 KB, patch)
2012-12-02 07:23 UTC, Ebru Akagündüz
reviewed Details | Review
online-accounts: Bind "accounts-button-add" to network availability (1.46 KB, patch)
2012-12-03 12:22 UTC, Debarshi Ray
committed Details | Review
online accounts: network error handling patch (1.78 KB, patch)
2012-12-03 15:07 UTC, Ebru Akagündüz
none Details | Review
online-accounts: Bind "accounts-tree-toolbutton-add" to network monitor (1.98 KB, patch)
2012-12-04 11:05 UTC, Debarshi Ray
none Details | Review
Disable adding accounts when network unavailable (1.97 KB, patch)
2012-12-04 13:26 UTC, Debarshi Ray
committed Details | Review
facebook on online account (745.21 KB, image/png)
2014-08-28 10:12 UTC, Jessie James Aton
  Details

Description Matthias Clasen 2012-06-15 12:44:02 UTC
I've noticed that the error handling when trying to add new accounts without network is somewhat uneven.

Google - I get an error dialog with a somewhat cryptic error message about an unexpected token

Microsoft Live - I get a 'couldn't load page' error inside the web view

I think we should have uniform error handling for all account types, and make it look nice.
Comment 1 Debarshi Ray 2012-06-20 09:34:53 UTC
Yes, we need to use the GNetworkMonitor APIs. There are some FIXMEs in the code for that.

Currently you get those different behaviours because OAuth and OAuth2 providers behave differently and hence fail differently. :-(
Comment 2 Allan Day 2012-11-08 16:01:05 UTC
Created attachment 228485 [details]
screenshot

Encountered this today trying to add my Facebook account.
Comment 3 Ebru Akagündüz 2012-11-29 10:09:52 UTC
Created attachment 230165 [details] [review]
network error handling patch
Comment 4 Ebru Akagündüz 2012-11-29 10:10:44 UTC
hi i added for solving this error.
Comment 5 Bastien Nocera 2012-11-29 15:18:28 UTC
Review of attachment 230165 [details] [review]:

Personally I would do:
monitor = g_network_monitor_get_default ();
And then use g_object_bind_property() to bind the "network-available" property from the monitor to the "sensitive" property of the widgets.

::: cc-online-accounts-panel.c
@@ +55,3 @@
+  GNetworkMonitor *monitor;
+  gboolean network_available;
+

Extraneous lifefeeds.

@@ +136,3 @@
 
+static void 
+netw_status(monitor){

Incomplete declaration.

@@ +138,3 @@
+netw_status(monitor){
+        network_available = g_network_monitor_get_network_available (monitor);
+        if(network_available){

Do:
if (g_network_monitor_get_network_available (monitor))
directly.

@@ +161,3 @@
   error = NULL;
+  panel->monitor = g_network_monitor_get_default ();
+  netw_status(monitor);

panel->monitor.
Comment 6 Ebru Akagündüz 2012-11-29 16:19:38 UTC
Created attachment 230210 [details] [review]
online accounts: network error handling

i changed it as your suggestion.
Comment 7 Bastien Nocera 2012-11-29 16:37:01 UTC
Comment on attachment 230210 [details] [review]
online accounts: network error handling

Use g_object_bind_property(), there's no need for a separate function (and please try to compile the code before submitting it).
Comment 8 Ebru Akagündüz 2012-12-01 10:39:59 UTC
Created attachment 230375 [details] [review]
network error handling patch

Hi, i added patch file. I used g_object_property()
Comment 9 Ebru Akagündüz 2012-12-01 12:49:13 UTC
I made a simple example with using g_object_bind_property(). Codes in the link are working. 
https://github.com/ebruAkagunduz/online-account/blob/master/playground/addAccount.c

 But i couldn't execute g_object_bind_property() for online-account.  May be i added g_object_property() wrong place for online-account.
Comment 10 Bastien Nocera 2012-12-01 19:26:47 UTC
Review of attachment 230375 [details] [review]:

Looks pretty good overall.

::: orjinal.c
@@ +142,3 @@
   GtkTreeIter iter;
 
+

No need to have that line feed here.

@@ +180,2 @@
   button = GTK_WIDGET (gtk_builder_get_object (panel->builder, "accounts-button-add"));
+  GNetworkMonitor *monitor;

Move the declaration to the top of the block please.

@@ +184,3 @@
+g_object_bind_property (monitor, "network-available",
+                          button, "sensitive",
+                          G_BINDING_SYNC_CREATE);

Use G_BINDING_DEFAULT instead, otherwise it won't be sync'ed at run-time.

@@ -768,3 @@
   add_account (panel);
 }
-

Another white space change that's not required.
Comment 11 Ebru Akagündüz 2012-12-02 07:23:05 UTC
Created attachment 230431 [details] [review]
network error handling patch

i changed it as your suggestion
Comment 12 Bastien Nocera 2012-12-03 07:29:06 UTC
Review of attachment 230431 [details] [review]:

A few more notes:

- Please read https://live.gnome.org/Git/Developers#Contributing_patches about contributing patches. The patch you've given lacks all data to commit it.
- The binding needs to be unref'ed when the panel is disposed (otherwise closing the panel and changing the network state will crash gnome-control-center).
Comment 13 Debarshi Ray 2012-12-03 12:21:21 UTC
Hey Ebru,

I modified the last version of your patch as per Bastien's comments, except the following:

- I restored the G_BINDING_SYNC_CREATE because otherwise if you start with network-available == FALSE, the button is not desensitized.

- Also, there is no need to unref the GBinding because the binding is automatically broken if either the source or target object used in the binding is finalized.

However, you left out "accounts-tree-toolbutton-add". It is the small "+" button on the left of the panel. The same thing needs to be done to toggle its sensitive property as the network changes. Can you please submit another patch to do that?

Now that you are a bit more familiar with the code, try reading https://live.gnome.org/Git/Developers#Contributing_patches (as Bastien pointed out) and see if you can use "git format-patch" to create your second patch.
Comment 14 Debarshi Ray 2012-12-03 12:22:11 UTC
Created attachment 230520 [details] [review]
online-accounts: Bind "accounts-button-add" to network availability
Comment 15 Debarshi Ray 2012-12-03 12:23:25 UTC
Comment on attachment 230520 [details] [review]
online-accounts: Bind "accounts-button-add" to network availability

commit ff186736dc2e063ee2df48a887ba394a20cd8d0e
Author: Ebru Akagunduz <ebru.akagunduz@gmail.com>
Date:   Mon Dec 3 13:14:34 2012 +0100

    online-accounts: Bind "accounts-button-add" to network availability
    
    Fixes: https://bugzilla.gnome.org/678161
Comment 16 Ebru Akagündüz 2012-12-03 15:07:21 UTC
Created attachment 230533 [details] [review]
online accounts: network error handling patch

hi, i changed it as your comment
Comment 17 Debarshi Ray 2012-12-04 11:05:56 UTC
Created attachment 230640 [details] [review]
online-accounts: Bind "accounts-tree-toolbutton-add" to network monitor
Comment 18 Debarshi Ray 2012-12-04 11:08:32 UTC
(In reply to comment #16)
> Created an attachment (id=230533) [details] [review]
> online accounts: network error handling patch
> 
> hi, i changed it as your comment

Well done. It works.

Two things:
 - there was a typo in your name, and I fixed it to "Ebru Akagunduz <ebru.akagunduz@gmail.com>"
 - I edited your commit message according to the format used elsewhere.
Comment 19 Debarshi Ray 2012-12-04 13:26:33 UTC
Created attachment 230653 [details] [review]
Disable adding accounts when network unavailable
Comment 20 Bastien Nocera 2013-04-19 08:59:53 UTC
Is there anything left to do here?
Comment 21 Jessie James Aton 2014-08-28 10:12:25 UTC
Created attachment 284670 [details]
facebook on online account

this happen when i try to signing in to facebook in online accounts.
Comment 22 Jessie James Aton 2014-08-28 10:14:19 UTC
Comment on attachment 284670 [details]
facebook on online account

this happen when i try to signing in to facebook in online accounts.
Comment 23 Georges Basile Stavracas Neto 2017-04-27 20:33:38 UTC
The Online Accounts redesign introduced code to detect the availability of network, an this issue likely won't happen anymore for we block all the UI when no network is available.

I'm closing this bug as FIXED, but feel free to reopen it if you see any other issues regarding network handling.