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)
: Andrew Overholt [:overholt]
Depends on: 764682 764928 766500
  Show dependency treegraph
Reported: 2012-06-20 02:15 PDT by Tim Guan-tin Chien [:timdream] (please needinfo)
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)
gal: review+
Details | Diff | Splinter Review

Description User image Tim Guan-tin Chien [:timdream] (please needinfo) 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 User image Blake Kaplan (:mrbkap) 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 User image 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 User image 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 User image cajbir (:cajbir) 2012-07-01 22:15:28 PDT
By "this patch" I mean this commit that landed:
Comment 6 User image Blake Kaplan (:mrbkap) 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.