Created attachment 185103 [details] [review]
network: Mobile broadband switch shows "OFF" after connecting

The Mobile broadband switch was not getting set to "ON" after a
connection completed, meaning that in order to disconnect it, you had
to select it twice (once to force it to correctly sync to "ON", and
then again to turn it off).

(In reply to comment #1)
> Created an attachment (id=185103) [details] [review]
> network: Mobile broadband switch shows "OFF" after connecting
> 
> The Mobile broadband switch was not getting set to "ON" after a
> connection completed, meaning that in order to disconnect it, you had
> to select it twice (once to force it to correctly sync to "ON", and
> then again to turn it off).

The patch is wrong. connected is not a NMClient.Device property, is an accessor property in NMDevice, and it should return true when the associated device is connected.

Is this happening with more than one mobile broadband device connected? Otherwise, it maybe a problem with tracking "wwan-enabled" on NMClient.Client.

Created attachment 185107 [details] [review]
network: Mobile broadband switch shows "OFF" after connecting

wacky. could have sworn I'd tested that.

Ok, multiple problems here. First, this._enabled is only true after the
toggle has been turned on, but we're using it here to decide whether or not
to turn the toggle on, which is no good.

Second, the enum value was wrong.

Even with these fixes though, it still doesn't work. Adding some log
statements shows that it's enabling the toggle switch on some different,
non-visible, nameless menu item. Not sure what's happening.

(In reply to comment #3)
> Created an attachment (id=185107) [details] [review]
> network: Mobile broadband switch shows "OFF" after connecting
> 
> wacky. could have sworn I'd tested that.
> 
> Ok, multiple problems here. First, this._enabled is only true after the
> toggle has been turned on, but we're using it here to decide whether or not
> to turn the toggle on, which is no good.
> 
> Second, the enum value was wrong.

Ok with the enum value, not with the _enabled stuff. Nothing connects to that "enabled-changed" (NMApplet connects to the more generic "state-changed"), so changing its signature has no effect.
_enabled means NMClient.wwan_enabled && NMClient.wwan_hardware_enabled, which is a prerequisite for a mobile broadband device reporting "connected", so the second fix should be ok.
Under no occasions NMDevice._enabled should be different to (NMClient.wwan_enabled && NMClient.wwan_hardware_enabled), and under no occasions it should happen that we get a NMDevice::state-changed without first a NMClient::notify::wwan-enabled.

> 
> Even with these fixes though, it still doesn't work. Adding some log
> statements shows that it's enabling the toggle switch on some different,
> non-visible, nameless menu item. Not sure what's happening.

There are two menu items involved here.
One is the "Mobile Broadband" subtitle item, which is NMApplet._devices.wwan.item, a NMWirelessSectionTitleMenuItem, whose toggle (if visible) tracks the properties on NMClient, and nothing else. The .connected property, as well as invoking .updateForDevice, has no effect on it, only on the status text.
The other is the device specific subtitle, which is NMDevice.statusItem, a NMDeviceTitleMenuItem, whose toggle, if operated, acts on NMDevice.activate / NMDevice.deactivate, and is updated by NMDevice._deviceStateChanged (with the additional constraint that Switch always changes its value, and expects the controller to revert). Because of bug 646074, the device specific subtitle has no text in it, but should be visible and with a switch anyway.
This means that there are two modes of operation, controlled by NMApplet._syncSectionTitle: if there are two or more wwan devices, the section subtitle item is set to track no device (which therefore shows the switch) and all the device subtitles are show, otherwise the device subtitles are hidden and the section subtitle shows the status text from the only device, or the switch.

It is a bit messy indeed, with three kind of switches all having slightly different semantics...

(Small update after testing... indeed, clicking on a connection has no effect on wwan_enabled, so the bug lies in NetworkManager...)

The WWAN enabled thing has been fixed in NM since:

commit 9152c304ff0f88308ae6176b8c2dddfb1988c434
Author: Dan Williams <dcbw@redhat.com>
Date:   Wed Apr 13 21:58:25 2011 -0500

    wwan: fix enabled state detection (bgo #647216)

*** This bug has been marked as a duplicate of bug 647216 ***