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 682929 - Don't use a global switch for all VPN connections
Don't use a global switch for all VPN connections
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: network-indicator
3.5.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 696431 (view as bug list)
Depends on:
Blocks: 683136
 
 
Reported: 2012-08-29 09:44 UTC by Allan Day
Modified: 2013-03-23 12:51 UTC
See Also:
GNOME target: ---
GNOME version: 3.5/3.6


Attachments
Mockup for VPN connections in the network menu (69.21 KB, image/png)
2012-08-29 09:44 UTC, Allan Day
  Details
Updated mockup - don't use a heading for VPN (71.95 KB, image/png)
2012-08-29 12:29 UTC, Allan Day
  Details
NetworkMenu: don't use a global switch for all VPN connections (22.62 KB, patch)
2012-08-29 14:39 UTC, Giovanni Campagna
none Details | Review
NetworkMenu: don't use a global switch for all VPN connections (23.72 KB, patch)
2012-08-31 20:09 UTC, Giovanni Campagna
none Details | Review
NetworkMenu: don't use a global switch for all VPN connections (23.86 KB, patch)
2012-10-11 19:18 UTC, Dan Winship
committed Details | Review

Description Allan Day 2012-08-29 09:44:10 UTC
Created attachment 222727 [details]
Mockup for VPN connections in the network menu

People are interested in individual VPN connections - that's what they want to turn on and off, not VPN in general. Also, it's a bit confusing to turn VPN on and then have one of the connections automatically activate.

It would be better to have on/off switches for each VPN network that can be individually toggled. See the attached mockup.
Comment 1 Allan Day 2012-08-29 12:29:34 UTC
Created attachment 222743 [details]
Updated mockup - don't use a heading for VPN

My original mockup was a bit inconsistent with how we handle other types of connections - with wired, we don't use a header for the type of interface, for example, and it's what I'd like to do for Mobile Broadband too (bug ). I think we should try to do the same here - it means the menu is a bit more compact, and it is consistent for different connection types.
Comment 2 Allan Day 2012-08-29 12:34:06 UTC
Missing bug number in the above comment - it should be bug 658230.
Comment 3 Giovanni Campagna 2012-08-29 14:37:56 UTC
(In reply to comment #1)
> Created an attachment (id=222743) [details]
> Updated mockup - don't use a heading for VPN
> 
> My original mockup was a bit inconsistent with how we handle other types of
> connections - with wired, we don't use a header for the type of interface, for
> example, and it's what I'd like to do for Mobile Broadband too (bug ). I think
> we should try to do the same here - it means the menu is a bit more compact,
> and it is consistent for different connection types.

Just a note: with wired we *do* use a header for the type of interface.
See the screenshot in bug 677142.

What happens is that device names are hidden when there is only one device, and that connection names are hidden when there is only one connection, which in the common case of autoconfigured eth0 results in "Wired [on|off]".
Comment 4 Giovanni Campagna 2012-08-29 14:39:54 UTC
Created attachment 222790 [details] [review]
NetworkMenu:  don't use a global switch for all VPN connections

Stop pretending that VPN is a NMDevice, and split the useful bits into
a NMConnectionBased interface.
Make each connection have its own switch menu item and handle its own
status, and remove the VPN section title, which is no longer needed.

This is above all the screenshield stuff, although rebasing should be possible.
This also breaks UI freeze, so I'm pushing it only if it's considered
high priority.
Comment 5 Giovanni Campagna 2012-08-31 20:09:30 UTC
Created attachment 223100 [details] [review]
NetworkMenu:  don't use a global switch for all VPN connections

Stop pretending that VPN is a NMDevice, and split the useful bits into
a NMConnectionBased interface.
Make each connection have its own switch menu item and handle its own
status, and remove the VPN section title, which is no longer needed.
Comment 6 Dan Winship 2012-10-11 19:16:57 UTC
Comment on attachment 223100 [details] [review]
NetworkMenu:  don't use a global switch for all VPN connections

>+        let pos = this._findConnection(vpnConnection.uuid);
>+        if (pos > 0) {

that needs to be >=

Other than that, everything seems to work, after some minor rebasing issues. I'm attaching an updated patch.
Comment 7 Dan Winship 2012-10-11 19:18:12 UTC
Created attachment 226289 [details] [review]
NetworkMenu: don't use a global switch for all VPN connections

Stop pretending that VPN is a NMDevice, and split the useful bits into
a NMConnectionBased interface.
Make each connection have its own switch menu item and handle its own
status, and remove the VPN section title, which is no longer needed.
Comment 8 Matthias Clasen 2012-10-30 02:16:58 UTC
Where do we stand on landing this ?
Comment 9 Dan Winship 2012-10-30 14:00:29 UTC
I believe the current patch is correct. If Giovanni is happy with it he should land it.
Comment 10 Giovanni Campagna 2012-10-30 15:13:40 UTC
Let's go then!
Comment 11 Giovanni Campagna 2013-03-23 12:51:34 UTC
*** Bug 696431 has been marked as a duplicate of this bug. ***