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 726401 - NetworkMenu: make sure menu icons are updated at the end of connectivity checks
NetworkMenu: make sure menu icons are updated at the end of connectivity checks
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: network-indicator
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2014-03-15 11:41 UTC by Giovanni Campagna
Modified: 2014-06-26 17:38 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
NetworkMenu: make sure menu icons are updated at the end of connectivity checks (3.95 KB, patch)
2014-03-15 11:41 UTC, Giovanni Campagna 		committed Details | Review

Description Giovanni Campagna 2014-03-15 11:41:03 UTC 
Icons inside the menu are updated only for device state change,
but for the main device they also depend on connectivity (which
is a global property).
Add a public method to force an update of the icon, and call it
when connectivity changes.
Comment 1 Giovanni Campagna 2014-03-15 11:41:06 UTC 
Created attachment 271995 [details] [review]
NetworkMenu: make sure menu icons are updated at the end of connectivity checks
Comment 2 Jasper St. Pierre (not reading bugmail) 2014-03-15 13:52:57 UTC 
Review of attachment 271995 [details] [review]:

When do they depend on connectivity? I can't see any getIndicatorIcon implementation using connectivity state.
Comment 3 Giovanni Campagna 2014-03-15 13:57:13 UTC 
(In reply to comment #2)
> Review of attachment 271995 [details] [review]:
> 
> When do they depend on connectivity? I can't see any getIndicatorIcon
> implementation using connectivity state.

_canReachInternet()
Comment 4 Jasper St. Pierre (not reading bugmail) 2014-03-15 14:01:04 UTC 
Review of attachment 271995 [details] [review]:

Ah. Can we put the signal handler in NMConnectionDevice then?

    this._client.connect('notify::connectivity', Lang.bind(this, this._sync));
Comment 5 Giovanni Campagna 2014-03-15 14:02:26 UTC 
(In reply to comment #4)
> Review of attachment 271995 [details] [review]:
> 
> Ah. Can we put the signal handler in NMConnectionDevice then?
> 
>     this._client.connect('notify::connectivity', Lang.bind(this, this._sync));

No, connectivity is a global property, and should be watched by global objects.
Comment 6 Jasper St. Pierre (not reading bugmail) 2014-03-15 14:04:13 UTC 
Really, I think that's a bad API on the part of NM -- connectivity really should be device-specific. And I really hate the iconChanged() method.
Comment 7 Giovanni Campagna 2014-03-15 14:07:37 UTC 
(In reply to comment #6)
> Really, I think that's a bad API on the part of NM -- connectivity really
> should be device-specific. And I really hate the iconChanged() method.

How about making _sync() public then?

And btw, connectivity is not device-specific, connectivity is a property of the default route: either you reach the internet in some way, or you don't.
It's totally useless (for us) to know that some network that is explicitly configured to be limited, is, well, limited.
Comment 8 Jasper St. Pierre (not reading bugmail) 2014-03-15 14:09:34 UTC 
On mobile devices where you're often using a combination of Wi-Fi, Wired and 4G, I think it's useful to know which devices are limited so you can balance your bandwidth and power usage.
Comment 9 Giovanni Campagna 2014-06-26 17:38:28 UTC 
Updated with review comments.

Attachment 271995 [details] pushed as 5f4591e - NetworkMenu: make sure menu icons are updated at the end of connectivity checks