Blake Kaplan (:mrbkap) (inactive) Assignee
	Description • 13 years ago

Right now, the synchronous wireless APIs allow a page to get the "connected" network which is a boolean state: connected or not. We need to expose state like "connecting", "obtaining IP address", etc.

	Fabien Cazenave [:kaze]
	Comment 1 • 13 years ago

Either that, or more boolean properties. I’m not sure which would be better.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 2 • 13 years ago

This patch renames connectedNetwork -> connectionStatus. My eventual hope is to also add things like the BSS/security used and IP address. Right now it contains the connected network (under "network") and the current status (under "status"). The status names are strings that map to the events that we currently fire.

This requires a couple of small changes to wifi.js.

	Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
	Comment 3 • 13 years ago

Comment on attachment 613221 [details] [diff] [review]
Proposed fix v1

>diff --git a/dom/wifi/nsIWifi.idl b/dom/wifi/nsIWifi.idl
>index 91b7022..b2ae6c9 100644
>--- a/dom/wifi/nsIWifi.idl
>+++ b/dom/wifi/nsIWifi.idl
>@@ -99,9 +99,11 @@ interface nsIDOMWifiManager : nsISupports {
>     readonly attribute boolean enabled;
> 
>     /**
>-     * A network object describing the currently connected network.
>+     * An object containing the following information:
>+     *  - status ("disconnected", "connecting", "associated", "connected")
>+     *  - network
>      */
>-    readonly attribute jsval connectedNetwork;
>+    readonly attribute jsval connectionStatus;
> 

I shall bikeshed this, because I can!

If we're going to change the interface here, how about calling this
|connection|, so that clients can say |connection.status| instead of
|connectionStatus.status|.

Also, the object returned here can never be null, right?  Should doc that.

It's a little confusing to return an object |o| in which |o.status =
"disconnected"| (e.g.) is a no-op, but I defer to DOM folks on API
conventions.

Implementation looks good, r=me with bikeshed painted.

	Fabien Cazenave [:kaze]
	Comment 4 • 13 years ago

(In reply to Blake Kaplan (:mrbkap) from comment #2)
> This patch renames connectedNetwork -> connectionStatus.

If this means that we’ll have:
  
 • `mozWifiManager.connection.network' returning a real network object (i.e. comparable with an element of the `getNetworks' output)

 • `mozWifiManager.connection.status' returning the state of the current/requested network

then I’m OK. As you mention, it will be easy to extend if we need more information (e.g. IP address, bandwidth…).

	Fabien Cazenave [:kaze]
	Comment 5 • 13 years ago

Another thing: I’d love to have an `onstatuschange' event to get along with the synchronous `status' property. There are a few use cases where this would be easier than a set of onconnect / onconnecting / ondisconnect / etc. event listeners and it would be more XHR-like.

	Fabien Cazenave [:kaze]
	Comment 7 • 13 years ago

>-  get connectedNetwork() {
>+  get status() {
>     if (!this._hasPrivileges)
>       throw new Components.Exception("Denied", Cr.NS_ERROR_FAILURE);
>-    return this._currentNetwork;
>+    return { status: this._connectionStatus, network: this._currentNetwork };
>   },

Does that mean we get this?
 • mozWifiManager.status.status
 • mozWifiManager.status.network

If 'status' and 'network' are properties of the same JS object, I’d prefer this object to carry a different name than 'status' (Chris suggested 'connection')… but that’s up to you, I’ll adapt the Gaia front-end accordingly when this lands.

	Mounir Lamouri (:mounir)
	Comment 8 • 13 years ago

Why not making this leave in navigator.mozConnection?
Like:
navigator.mozConnection.wifi.status
navigator.mozConnection.wifi.network // or .ssid, don't know what .network is for exactly

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 9 • 13 years ago

(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #8)
> Why not making this leave in navigator.mozConnection?

The entirety of mozWifi(Manager) should live under mozConnection, IMO. gal and I actually talked about that in the beginning. I'll file a followup on doing that.

> navigator.mozConnection.wifi.network // or .ssid, don't know what .network
> is for exactly

I'd prefer keeping ...network.ssid. That'll make it easier to add any additional information (like what security is available) later.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 10 • 13 years ago

I grabbed the wrong brush the last time.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 11 • 13 years ago

https://hg.mozilla.org/mozilla-central/rev/14084207e9d1

	Philipp von Weitershausen [:philikon]
	Comment 12 • 13 years ago

(In reply to Fabien Cazenave (:kaze) from comment #5)
> Another thing: I’d love to have an `onstatuschange' event to get along with
> the synchronous `status' property. There are a few use cases where this
> would be easier than a set of onconnect / onconnecting / ondisconnect / etc.
> event listeners and it would be more XHR-like.

+1. File it?

(In reply to Blake Kaplan (:mrbkap) from comment #9)
> (In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #8)
> > Why not making this leave in navigator.mozConnection?
> 
> The entirety of mozWifi(Manager) should live under mozConnection, IMO. gal
> and I actually talked about that in the beginning. I'll file a followup on
> doing that.

In that case, we should perhaps move navigator.mozMobileConnection to .mozConnection.mobile as well. But I want to do that in a follow-up, too :)

	Philipp von Weitershausen [:philikon]
	Comment 13 • 13 years ago

(In reply to Philipp von Weitershausen [:philikon] from comment #12)
> (In reply to Fabien Cazenave (:kaze) from comment #5)
> > Another thing: I’d love to have an `onstatuschange' event to get along with
> > the synchronous `status' property. There are a few use cases where this
> > would be easier than a set of onconnect / onconnecting / ondisconnect / etc.
> > event listeners and it would be more XHR-like.
> 
> +1. File it?

Never mind... bug 745114 :)