Created attachment 185351 [details] [review]
Hanlde all device states

Comment on attachment 185351 [details] [review]
Hanlde all device states

>+        case NetworkManager.DeviceState.UNMANAGED:
>+            return _("device not managed");

this would break string freeze.

Is a device ever visible in the UNMANAGED state for more than an instant? Bug 646708 patched this to just return null in the UNMANAGED case, as with DISCONNECTED.

>+        case NetworkManager.DeviceState.DEACTIVATING:
>+            return _("disconnecting...");

ok, so it breaks string freeze even without the other change.

(In reply to comment #2)
> (From update of attachment 185351 [details] [review])
> >+        case NetworkManager.DeviceState.UNMANAGED:
> >+            return _("device not managed");
> 
> this would break string freeze.
>>
> Is a device ever visible in the UNMANAGED state for more than an instant? Bug
> 646708 patched this to just return null in the UNMANAGED case, as with
> DISCONNECTED.

Bug 646708 is rigth, unmanaged devices are automatically hidden synchronously when state changes.
(getStatusLabel is still called internally, that's why you see the warning)

> >+        case NetworkManager.DeviceState.DEACTIVATING:
> >+            return _("disconnecting...");
> 
> ok, so it breaks string freeze even without the other change.

Is this already used by NetworkManager?
I was told that nothing in NM 0.9 uses it, and deactivation is instantaneous.

Nothing uses DEACTIVATING yet, but at some point we will and it might be wise to handle the state already so we don't have to fix it up later?  It's your call really, I just tossed that in as a bonus.

Devices will be unmanaged in 3 cases:

1) immediately after being recognized before they are managed by NM, if the device is really going to be managed by NM it won't be in the unmanaged state for long

2) if the user has specifically unmanaged a device from NM's control, which sometimes happens, and we accounted for this specifically in nm-applet because people would often wonder why NM couldn't see their device, and that's because it was unmanaged either by them manually or automatically by the installer for some reason

3) for a very short period of time when NM is shutting down or when the device is disconnected

The major reason nm-applet showed devices in unmanaged state was #2.  We got a *lot* of questions with 0.6 about why people's devices weren't shown or recognized by NM, and changed the menu to show devices as unmanaged so that people had a clue what was going on.  Otherwise if somehow the device is unmanaged, you have no idea why and you have to start asking questions, and frankly it was getting really annoying on the list to have to keep explaining it over and over again.  It's a rare enough case that it would not impact the normal usage of the applet, most users won't see it, but the ones that do have unmanaged devices will get the clue and don't have to ask.

Review of attachment 185351 [details] [review]:

Ok, so the part for IP_CHECK and SECONDARIES is good to commit right now (3.0.1), but the part on DEACTIVATING can wait for 3.2, given that it breaks string freeze.
Same for UNMANAGED I think, which also requires changing NMApplet.prototype._syncSectionTitle not to filter out unmanaged devices.
If showing unmanaged devices is a must for 3.0.1, we can show "unavailable" or the empty string.

Created attachment 186283 [details] [review]
network: add a few more states to getStatusLabel()

The IP_CHECK and SECONDARIES states should be considered part of the
"connecting..." phase.

DEACTIVATING should be its own stage, but that would break string
freeze, so we just treat it like DISCONNECTED for now.

UNMANAGED needs to be treated differently in 3.2, but it is too late
to fix it for 3.0.1.

=======

It's too late to change the way the menu handles UNMANAGED devices;
if we stop filtering them out, there's guaranteed to be all sorts of
edge cases that don't work properly.

Review of attachment 186283 [details] [review]:

Looks good to me for 3.0.1. Don't forget to fix for 3.2 after branching.

Review of attachment 186283 [details] [review]:

Looks good to me for 3.0.1

Comment on attachment 186283 [details] [review]
network: add a few more states to getStatusLabel()

Attachment 186283 [details] pushed as 7e857de - network: add a few more states to getStatusLabel()

removing from 3.0.1 blocker, but leaving the bug open to fix better for 3.2

Created attachment 187510 [details] [review]
NetworkMenu: show devices that are unmanaged

Some users are confused when their devices are not shown in the
network menu, even if they configured them manually. Mark their presence
by showing them in the menu, even if they cannot be otherwise
interacted with.
Also add a status string for deactivating devices (none currently,
soon will appear in NetworkManager).

Attachment 187510 [details] pushed as dbd629d - NetworkMenu: show devices that are unmanaged