Last Comment Bug 766497 - wifiManager.enabled value is not "synced" across the frame
: wifiManager.enabled value is not "synced" across the frame
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
Depends on: 764682 764928 766500
  Show dependency treegraph
Reported: 2012-06-20 02:15 PDT by Tim Guan-tin Chien [:timdream] (please needinfo; on leave in July)
Modified: 2012-07-02 15:31 PDT (History)
11 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Proposed fix (12.25 KB, patch)
2012-06-28 03:08 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
gal: review+
Details | Diff | Review

Description Tim Guan-tin Chien [:timdream] (please needinfo; on leave in July) 2012-06-20 02:15:28 PDT
+++ This bug was initially created as a clone of Bug #764682 +++


1. Install latest Gaia
2. Go to Settings app, try to turn on or off wifi
3. Pull down the Utility Tray from the status bar
4. Observe the Quick Setting buttons of Wifi


1. Each time you pull down the tray, the button will sync it's status with wifiManager.enabled. It should be either red or green depend on the same toggle button on Settings app.


1. the button will always stay Off or On (red or green) no matter what you do. You can actually get wifiManager.connection.status = 'connected' but wifiManager.enabled = false!


The reverse does not apply since Settings app use different values to check the status of wifiManager, and you can effectively "sync" the value by kill the Settings app and launch it again.
Comment 1 Blake Kaplan (:mrbkap) (please use needinfo!) 2012-06-28 03:08:25 PDT
Created attachment 637436 [details] [diff] [review]
Proposed fix

Getting the "multiple requests at the same time" code was tricky... I have tested this to the best of my ability, and the gaia code will have to take advantage of the new notifications itself.

With this patch, calls to setEnabled will send *two* notifications (one with the request and the event). I'm not terribly happy with that, but given that the request to setEnabled can fail, there is no better solution.
Comment 2 Andreas Gal :gal 2012-06-29 01:55:13 PDT
Comment on attachment 637436 [details] [diff] [review]
Proposed fix

Review of attachment 637436 [details] [diff] [review]:

::: dom/wifi/WifiWorker.js
@@ +864,4 @@
>    // Public interface of the wifi service
>    manager.setWifiEnabled = function(enable, callback) {
> +    if ((enable && manager.enabled) ||

enable == manager.enabled
Comment 4 cajbir (:cajbir) 2012-07-01 22:13:50 PDT
I'm getting the following in my logcat:

E/GeckoConsole(  252): [JavaScript Error: "enabled is not defined" {file: "jar:file:///system/b2g/omni.ja!/components/WifiWorker.js" line: 867}]

Should the 'enable' in this patch have been 'enabled'?
Comment 5 cajbir (:cajbir) 2012-07-01 22:15:28 PDT
By "this patch" I mean this commit that landed:
Comment 6 Blake Kaplan (:mrbkap) (please use needinfo!) 2012-07-02 05:15:11 PDT fixes comment 4. Thanks for pointing it out.

Note You need to log in before you can comment on or make changes to this bug.