Closed
Bug 1123680
Opened 9 years ago
Closed 9 years ago
Forget Wifi cause problems to reconnect on it immediately
Categories
(Firefox OS Graveyard :: Wifi, defect)
Tracking
(blocking-b2g:2.1+, firefox39 wontfix, firefox40 wontfix, firefox41 fixed, b2g-v2.0 unaffected, b2g-v2.1 verified, b2g-v2.1S fixed, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: clement.lefevre, Assigned: amchung)
References
Details
Attachments
(2 files, 10 obsolete files)
2.25 KB,
patch
|
hchang
:
review+
jocheng
:
approval-mozilla-b2g34+
jocheng
:
approval-mozilla-b2g37+
|
Details | Diff | Splinter Review |
2.84 MB,
video/3gpp
|
Details |
Forgetting a Wifi network can lead to difficult reconnect to this same network. You need to have a network you are connected to. If you forget this network, and then want to connect to this one again, then status will not be good: - At a first time, status in the list will show "offline" for a short or longer time. - Then you ask to connect again to the network. The phone trigger the screen asking for password. You enter it. It is registering the network with the password, but in the "manage networks" interface, it show it as out of range. The notification bar seems to be associated with the settings app: in settings you still have the icon that shows you you are connected to a wifi, but if you get out of the settings app you do not have this icon. To solve this problem, you need to manually search again to have the phone connecting to the network (or put it on sleep and then wake it up, or reboot). Instead, status should be updated immediately.
Reporter | ||
Comment 1•9 years ago
|
||
See maybe this old bug: https://bugzilla.mozilla.org/show_bug.cgi?id=802565
Reporter | ||
Comment 2•9 years ago
|
||
Reproduced on: Build ID 20150119010205 Build Type user Gaia Revision 0f65b258bceddd9d479b3c027d9bd234c1e99aaf Gaia Date 2015-01-16 22:12:30 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/6446c26b45f9 Gecko Version 38.0a1 Device ID flame Firmware(Release) 4.4.2 Firmware(Incremental) 39 Firmware Date Thu Oct 16 18:19:14 CST 2014 Bootloader L1TC00011880
Reporter | ||
Updated•9 years ago
|
Summary: Forget Wifi cause to problems to reconnect on it immediately → Forget Wifi cause problems to reconnect on it immediately
Updated•9 years ago
|
QA Contact: bzumwalt
Comment 4•9 years ago
|
||
Issue DOES reproduce on Flame 2.1, 2.2, and 3.0 After forgetting connected network, status of network shows as "offline" very briefly. If user tries to reconnect immediately after this by tapping network and entering password, the status bar shows wifi disabled and the network shows as out of range in manage networks screen. Tapping search again button resolves these issues. (Found that forgetting network must be done by clicking network in list, as user must immediately click on network in list after "Offline" status disappears.) On 2.1 the Manage Networks screen the status shows as connected instead of out of range, but all other issues listed such as the status bar and the short offline message reproduce. Device: Flame 2.1 Build ID: 20150123001230 Gaia: 234ec27050481f4787f1f4750ec26f62ce5cc2c0 Gecko: 4cdcc0e85fc0 Gonk: e7c90613521145db090dd24147afd5ceb5703190 Version: 34.0 (2.1) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Device: Flame 2.2 Build ID: 20150123002505 Gaia: 237008137f6d72b9cad25ff4faff14ff2c40ac63 Gecko: be24dd482a83 Gonk: e7c90613521145db090dd24147afd5ceb5703190 Version: 37.0a2 (2.2) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0 Device: Flame 3.0 Build ID: 20150123010227 Gaia: cba2f0bf49b882e0044c3cc583de8fcf83d2ffa4 Gecko: 494632b9afed Gonk: e7c90613521145db090dd24147afd5ceb5703190 Version: 38.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0 Issue does NOT reproduce on Flame 2.0 After forgetting network, the status shows as "Secured by WPA-PSK" and reconnection proceeds without difficulty. Device: Flame 2.0 Build ID: 20150123000204 Gaia: 2989f2b2bd12fcc0e9c017d2db766e76a55873b8 Gecko: bd197cfd10a7 Gonk: e7c90613521145db090dd24147afd5ceb5703190 Version: 32.0 (2.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → affected
status-b2g-master:
--- → affected
Flags: needinfo?(ktucker)
Keywords: qawanted → regression
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Comment 5•9 years ago
|
||
[Blocking Requested - why for this release]: Regression which might confuse an end user.
Status: UNCONFIRMED → NEW
blocking-b2g: --- → 2.1?
Ever confirmed: true
Comment 6•9 years ago
|
||
Not sure if this is related to Bug 1100283. Ni myself and come back to this later.
Flags: needinfo?(vchang)
Updated•9 years ago
|
Assignee: nobody → vchang
Comment 8•9 years ago
|
||
Amy, please help to check this one.
Assignee: vchang → amchung
Flags: needinfo?(vchang)
Comment 9•9 years ago
|
||
I just reproduced this on master
Assignee | ||
Comment 10•9 years ago
|
||
I tried the latest version and I can't reproduce this situation. My testing steps are: 1. connect to the any wifi AP. 2. To Wifi Management, forget this wifi connection. 3. back to wifi setting. 4. confirm the wifi AP that you connected is not offline. 5. connect again to the network, then in the "manage networks" interface, it doesn't show it as out of range. Do I neglect any steps to reproduce?
Comment 11•9 years ago
|
||
Please note, as per comment 4, you must forget network from network list NOT manage networks screen in order for STR to be viable as re-signing in to access point must be done quickly after forgetting network. This issue does reproduce on latest Flame 3.0 After forgetting, then quickly signing back into access point the status bar shows no wifi, user receives "No internet connection" message when trying to search via rocketbar. The "out of range" message no longer appears, but the rest of the bug reproduces 100% Device: Flame 3.0 Master BuildID: 20150213010213 Gaia: 2a2b008f9ae957fe19ad540d233d86b5c0b6829e Gecko: 2f5c5ec1a24b Version: 38.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Assignee | ||
Comment 12•9 years ago
|
||
Dear Brogan, I used the forget function in AP status panel, but the AP showed the offline just very short time(1~2secs) and didn't show "out of range" when I used search button. When I used forget function and return to wifi setting panel, I keep pressed the AP that I connected then the offline status will show longer. Do I miss any reproduces steps? Thanks!
Keywords: qawanted
Comment 13•9 years ago
|
||
Video reference: http://youtu.be/vixWT2UUAYs Amy, clicking to reconnect after forgetting must be done fairly quickly for this to reproduce. As I mentioned in comment 11, I am not seeing the "Out of range" message anymore either, but the rest of the bug (such as not actually being connected to wifi) still occurs as shown in the above video.
Comment 14•9 years ago
|
||
Reproduced on Flame 3.0 After forgetting, then quickly signing back into access point the status bar shows no wifi, user receives "No internet connection" message when trying to search via rocketbar. Device: Flame 3.0 Build ID: 20150216010344 Gaia: f0b93e0668ef9565bd6f050b15b4f794d59feb65 Gecko: e0cb32a0b1aa Version: 38.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
Reporter | ||
Comment 15•9 years ago
|
||
I got behaviors linked to this couple of days ago: connected to my personal wifi, I put the phone to sleep and went to another place with a wifi already registered without using the phone at all between these places. When I woke up the phone at the second place, the status was not refreshed at all: notifications was showing that it was connected to a wifi (I thought it was connected to the second wifi, from the second place), but when trying to do something needing Internet it failed. So it appears it was displaying it was connected to the wifi from the first place (home) while it was not anymore, and it was preventing he phone from connecting to another wifi until it refresh its status.
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Assignee | ||
Comment 16•9 years ago
|
||
Dear Vincent, I found at associate function, wifi do reconnect when wifi state needs to in DISCONNECTED and SCANNING in WifiWorker.js. But I print the wifi state at associate function, the wifi state shows INACTIVE in every status. And if the wifi can connected successfully, then wifi state from INACTIVE changes to SCANNING. In the fail case, the wifi state in INACTIVE, then debug console shows "E/HWComposer( 211): Non-uniform vsync interval: 100087345". I will trace this message and flow. Thanks!
Flags: needinfo?(vchang)
Comment 17•9 years ago
|
||
The reason we use reconnect in DISCONNECTED or SCANNING state is reconnect only takes effect if already disconnected. I think we can use reassociate when wifi is in others state. Not sure any side effect though. Any ideas how wifi state is in INACTIVE in failed case?
Flags: needinfo?(vchang)
Assignee | ||
Comment 18•9 years ago
|
||
I found when wifi state in INACTIVE and called associate function from WifiManager, possibility debug console showed "Already associated - do noting" from wpa supplicant. In this case, it needed to wait wpa supplicant scanning. For avoid this situation, I tried to do three action in INACTIVE state: 1. reconnect 2. return false, gaia shows some error message. 3. reassociate And case 3 could fix issue, but it still exists connection fail and reconnecting continuously. I will confirm what's different between associate function and reassociate function in wpa supplicant.
Assignee | ||
Comment 19•9 years ago
|
||
Attachment #8581510 -
Flags: review?(hchang)
Assignee | ||
Comment 20•9 years ago
|
||
Comment on attachment 8581510 [details]
diff_1123680
Dear Henry,
Please confirm this patch, and ignore my debug message.
Thanks!
Amy
Comment 21•9 years ago
|
||
I am afraid of calling "this.forget(msg)" directly would send unexpected message 'WifiManager:forget:Return' back to DOMWifiManager.
Assignee | ||
Comment 22•9 years ago
|
||
Attachment #8581510 -
Attachment is obsolete: true
Attachment #8581510 -
Flags: review?(hchang)
Attachment #8582297 -
Flags: review?(hchang)
Assignee | ||
Comment 23•9 years ago
|
||
Dear Henry, Please confirm this patch that I changed to use removeNetwork function. Thanks! Amy
Assignee | ||
Comment 24•9 years ago
|
||
Dear Henry, I changed the patch file, please confirm it. Thanks! Amy
Attachment #8582297 -
Attachment is obsolete: true
Attachment #8582297 -
Flags: review?(hchang)
Attachment #8582866 -
Flags: review?(hchang)
Comment 25•9 years ago
|
||
Comment on attachment 8582866 [details] [diff] [review] 0001-Bug-1123680-Forget-Wifi-cause-problems-to-reconnect-.patch Review of attachment 8582866 [details] [diff] [review]: ----------------------------------------------------------------- Thanks Amy. I would r+ it after confirming the netowkrKey is always valid and fixing the way you call wifi commands. Thanks! ::: dom/wifi/WifiWorker.js @@ +3235,1 @@ > self._sendMessage(message, ok, ok, msg); It seems no change around here, doesn't it? @@ +3239,5 @@ > + // That maybe occurs wpa supplicant that suppose already associated. > + // To avoid this case, need to clear AP info and call reassoiate > + // in INACTIVE state. > + let networkKey = getNetworkKey(network); > + let configured = this.configuredNetworks[networkKey]; I'm sure if there would be invalid networkKey case. What do you think? @@ +3240,5 @@ > + // To avoid this case, need to clear AP info and call reassoiate > + // in INACTIVE state. > + let networkKey = getNetworkKey(network); > + let configured = this.configuredNetworks[networkKey]; > + WifiManager.removeNetwork(configured.netId, function() {}); Even though the wifi commands are executed sequentially, it would be better to code like the following: WifiManager.removeNetwork(configured.netId, function() { WifiManager.reassociate(function() {}) });
Attachment #8582866 -
Flags: review?(hchang)
Assignee | ||
Comment 26•9 years ago
|
||
Dear Henry, I have modified this patch, please review it. Thanks!
Attachment #8582866 -
Attachment is obsolete: true
Attachment #8582922 -
Flags: review?(hchang)
Comment 27•9 years ago
|
||
Comment on attachment 8582922 [details] [diff] [review] 0001-Bug-1123680-Forget-Wifi-cause-problems-to-reconnect-.patch Review of attachment 8582922 [details] [diff] [review]: ----------------------------------------------------------------- Hi Amy, last two tiny issue in the patch. Thanks! ::: dom/wifi/WifiWorker.js @@ +3240,5 @@ > + // To avoid this case, need to clear AP info and call reassoiate > + // in INACTIVE state. > + let networkKey = getNetworkKey(network); > + if (!(networkKey in this.configuredNetworks)) { > + this._sendMessage(message, false, "Trying to forget an unknown network", msg); The error message needs to be changed. @@ +3245,5 @@ > + return; > + } > + let configured = this.configuredNetworks[networkKey]; > + WifiManager.removeNetwork(configured.netId, function() { > + WifiManager.reassociate(function() {}) Just realized we need to call "self._sendMessage(message, ok, ok, msg);" to report success here.
Assignee | ||
Comment 28•9 years ago
|
||
Dear Henry, I have modified this patch, pleas help me to review it. Thanks!
Attachment #8582922 -
Attachment is obsolete: true
Attachment #8582922 -
Flags: review?(hchang)
Attachment #8582946 -
Flags: review?(hchang)
Assignee | ||
Comment 29•9 years ago
|
||
Dear Henry, I have added sendMessage, please help me to review it. Thanks!
Attachment #8582946 -
Attachment is obsolete: true
Attachment #8582946 -
Flags: review?(hchang)
Attachment #8583668 -
Flags: review?(hchang)
Comment 30•9 years ago
|
||
Comment on attachment 8583668 [details] [diff] [review] 0001-Bug-1123680-Forget-Wifi-cause-problems-to-reconnect-.patch Review of attachment 8583668 [details] [diff] [review]: ----------------------------------------------------------------- Good job Amy! Thanks :)
Attachment #8583668 -
Flags: review?(hchang) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: regression → checkin-needed
Comment 31•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/cfdeb23e7296
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/cfdeb23e7296
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S9 (3apr)
Comment 33•9 years ago
|
||
Could this have caused bug 1148453? In that case, can this be backed out, please?
Comment 34•9 years ago
|
||
(In reply to Martijn Wargers [:mwargers] (QA) from comment #33) > Could this have caused bug 1148453? In that case, can this be backed out, > please? I tested by reverting the patch for this bug on the latest gecko, (commit 6e1a7d81f6d6481f5b9e31605167d483f05e12b8) and the problem did not reproduce, while with the plain gecko I was able to repro Bug 1148453, so this patch seems to be the culprit.
We're investigating that bug, but it seems to be an issue where the success branch of the mozRequest isn't called after connection. Any chance we moved stuff around so it's trying to do a callback across a process boundary or something? This is the code: 263 connectToWiFi: function(aNetwork, aCallback) { 264 var callback = aCallback || marionetteScriptFinished; 265 var manager = window.navigator.mozWifiManager; 266 267 if (this.isWiFiConnected(aNetwork)) { 268 console.log('already connected to network with ssid \'' + 269 aNetwork.ssid + '\''); 270 callback(true); 271 } 272 else { 273 var req; 274 if (window.MozWifiNetwork === undefined) { 275 req = manager.associate(aNetwork); 276 } else { 277 req = manager.associate(new window.MozWifiNetwork(aNetwork)); 278 } 279 280 req.onsuccess = function() { 281 console.log('waiting for connection status \'connected\''); 282 waitFor( 283 function() { 284 console.log('success connecting to network with ssid \'' + 285 aNetwork.ssid + '\''); 286 callback(true); 287 }, 288 function() { 289 console.log('connection status: ' + manager.connection.status); 290 return manager.connection.status === 'connected'; 291 } 292 ); 293 }; 294 295 req.onerror = function() { 296 console.log('error connecting to network ' + req.error.name); 297 callback(false); 298 }; 299 } 300 },
correcting myself, I meant that the DOMRequest for mozWifiManager.associate() may not be calling the onsuccess callback.
OK, there's probably at least one bug in the patch. We tried fixing it in a local build and briefly saw the behavior resolve, but then it came back. Still, worth pointing out the bug, in case it's involved: > + WifiManager.reassociate(function() { > + self._sendMessage(message, ok, ok, msg); > + }); The first line should supply ok as a parameter to the function() callback, since that's what doBooleanCommand will populate when it resolves the WifiCommand. Otherwise this code will pick up ok out of the closure, and possibly send false in the message even though the reassociate actually succeeded. See: > WifiManager.reconnect(function (ok) { > self._sendMessage(message, ok, ok, msg); > }); for comparison. As I said, fixing this didn't appear to fix the whole issue. However, backing the patch out did per comment 34. I'd like to renew the request for that backout for this fix and whatever else might be the problem here.
Flags: needinfo?(amchung)
Keywords: qablocker
Comment 38•9 years ago
|
||
Backed out: https://hg.mozilla.org/integration/b2g-inbound/rev/9d6212710784
https://hg.mozilla.org/mozilla-central/rev/9d6212710784
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Comment 40•9 years ago
|
||
Issue DOES reproduce on the latest Flame 3.0 After forgetting wifi connection, reconnecting to same is not successful. User has to restart settings app or reboot phone to connect to wifi. Youtube link: http://youtu.be/E30EMTI7w_k Device: Flame 3.0 Build ID: 20150401010204 Gaia: 03164bd160809747e6a198e0dba1b7c3ee7789f5 Gecko: 18a8ea7c2c62 Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b Version: 40.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Comment 41•9 years ago
|
||
May be related to bug 1145068 as per https://bugzilla.mozilla.org/show_bug.cgi?id=1145068#c6
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage?][failed-verification]
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?][failed-verification] → [QAnalyst-Triage+][failed-verification]
Flags: needinfo?(ktucker)
Assignee | ||
Comment 43•9 years ago
|
||
Dear Martijn, I tried gaia-ui-test for 1123680 patch, and I am sure the testing item test_browser_lan is pass after added this patch. So would you test the fail testing items on the build including my patch individually? Or report the full testing after opened wifi debug message. Thanks!
Flags: needinfo?(amchung) → needinfo?(martijn.martijn)
Comment 44•9 years ago
|
||
This was backed out on mozilla-central: http://hg.mozilla.org/mozilla-central/rev/9d6212710784 Amy, at this point I don't have my own build yet, which I could push on to the Flame device and test with, I'll do that, but it'll take some time. But did you see comment 34 on this bug? No-Jun specifically tested backing out this patch and he said it's the cause of bug 1148453.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 45•9 years ago
|
||
Ok, chatted with No-Jun on irc and he's sure this patch caused the issues in bug 1148453, including the test_browser_lan issues. Are you sure you built gecko and pushed it to the Flame device after you applied the patch?
Flags: needinfo?(martijn.martijn) → needinfo?(amchung)
Comment 46•9 years ago
|
||
Just to add a background info here: the Bug 1148453 would only occur when two marionette scripts both using wifi connection runs one after another. And when you backed out your patch and tested, the error you saw was actually related to the different bug, but the wifi error no longer appeared. I also tested locally by reverting this patch, and saw the wifi bug disappear, so we're pretty certain that it was the cause of our test failure. But, with the patch backed out, while it would fix Bug 1148453, it would also reopen this bug. What we might need to decide is that whether we should use this patch, and fix marionette code, or leave marionette code and submit a new patch. As for how marionette handles wifi connection request, geo would be able to provide you with the more info.
Assignee | ||
Comment 47•9 years ago
|
||
Dear Geo, I can't duplicate the mariontte test error as your testing result(Wifi connection time out). According to this error, I have tested connection time on adding my patch and removing my patch. The average connection time is 4 secs to 5 secs by adding my patch, and the average connection time is 3 secs by removing my patch. I experimented to compare the boundary of connection time out is 60 secs and increasing boundary to 120 secs, and the testing results are same, it doesn't occur Wifi connection time out. The contingency error of testing results occur on go_to_url time out, but the contingency error is misjudged. Please attach the log of testing result and would you give me some suggestion to modify marionette code for passing marionette test? Thanks!
Flags: needinfo?(amchung) → needinfo?(gmealer)
Comment 48•9 years ago
|
||
Per offline talk, we should not use removeNetwork here. We should find the other way to fix the problem.
Assignee | ||
Comment 49•9 years ago
|
||
Dear Henry, Please help me to confirm my patch. Thanks!!
Attachment #8583668 -
Attachment is obsolete: true
Attachment #8597061 -
Flags: review?(hchang)
Assignee | ||
Comment 50•9 years ago
|
||
Attachment #8597061 -
Attachment is obsolete: true
Attachment #8597061 -
Flags: review?(hchang)
Attachment #8597063 -
Flags: review?(hchang)
(In reply to Amy Chung from comment #47) > Dear Geo, > I can't duplicate the mariontte test error as your testing result(Wifi > connection time out). > According to this error, I have tested connection time on adding my patch > and removing my patch. > The average connection time is 4 secs to 5 secs by adding my patch, and the > average connection time > is 3 secs by removing my patch. > I experimented to compare the boundary of connection time out is 60 secs and > increasing boundary to 120 secs, and the testing results are same, it > doesn't occur Wifi connection time out. > The contingency error of testing results occur on go_to_url time out, but > the contingency error is misjudged. > Please attach the log of testing result and would you give me some > suggestion to modify marionette code for passing marionette test? > > Thanks! Amy, Not sure if this is relevant anymore, so please do advise. It would be useful if you test (new method as well) with something that runs multiple wifi tests in a row, since that's what sparked the error condition before--probably interfered with resetting in between tests. I'm NI'ing No-Jun to give a suggestion as to which suite would be a good one to expose that, since he was the one helping out there before. I do understand that might be too onerous given that we don't typically have devs pre-test against the QA acceptance suite. As long as you're responsive to any problems that come up, I'm generally OK handling them after the fact. Just be aware this is very central functionality for us.
Flags: needinfo?(npark)
Flags: needinfo?(gmealer)
Assignee | ||
Comment 52•9 years ago
|
||
Attachment #8598552 -
Flags: review?(hchang)
Assignee | ||
Comment 53•9 years ago
|
||
Comment on attachment 8598552 [details] [diff] [review] forget_diff Dear Henry, Please help me to review the patch. Thanks!
Assignee | ||
Updated•9 years ago
|
Attachment #8598552 -
Attachment description: diff_1123680 → forget_diff
Assignee | ||
Updated•9 years ago
|
Attachment #8597063 -
Attachment is obsolete: true
Attachment #8597063 -
Flags: review?(hchang)
Comment 54•9 years ago
|
||
Comment on attachment 8598552 [details] [diff] [review] forget_diff Review of attachment 8598552 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/wifi/WifiWorker.js @@ +686,4 @@ > // 2. current network if no SSID is provided, it's not guaranteed that > // current network matches requested SSID. > if ((!ssid && network.status === "CURRENT") || > + (ssid && network.ssid && ssid === network.ssid)) { We might need to figure out what's wrong with |dequote| here. @@ +3298,5 @@ > > + function connectToNetwork() { > + WifiManager.updateNetwork(privnet, (function(ok) { > + if (!ok) { > + this._sendMessage(message, false, "Network is misconfigured", msg); |this| may be not as expected since this is a callback. We need to use |self| or arrow function or binding this to the callback instead.
Assignee | ||
Comment 55•9 years ago
|
||
Dear Henry, Please help me to confirm this patch.
Attachment #8598552 -
Attachment is obsolete: true
Attachment #8598552 -
Flags: review?(hchang)
Attachment #8599193 -
Flags: review?(hchang)
Comment 56•9 years ago
|
||
Comment on attachment 8599193 [details] [diff] [review] 0001-Bug-1123680-Forget-Wifi-cause-problems-to-reconnect-.patch Review of attachment 8599193 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me except for the extra white spaces. Thanks! ::: dom/wifi/WifiWorker.js @@ +687,4 @@ > // current network matches requested SSID. > if ((!ssid && network.status === "CURRENT") || > (ssid && network.ssid && ssid === dequote(network.ssid))) { > + return callback(net); There is no change here.
Attachment #8599193 -
Flags: review?(hchang) → review+
Comment 57•9 years ago
|
||
Because the Gaia UI tests wipe settings each time it runs the test that requires wifi connection, there is no specific tests around forgetting and reconnecting the wifi. If tests like test_settings_wifi.py (under functional/settings folder) can be executed repeatedly, say 5 times (with the parameter --repeat 5), then I think that would be sufficient.
Flags: needinfo?(npark)
Comment 58•9 years ago
|
||
Comment 59•9 years ago
|
||
Comment on attachment 8605028 [details] [diff] [review] Bug1123680.patch Carry r+ from previous review
Attachment #8605028 -
Flags: review+
Updated•9 years ago
|
Attachment #8599193 -
Attachment is obsolete: true
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 60•9 years ago
|
||
Dear No-Jun, I have tested test_settings_wifi.py for 5 times, and the testing results are pass. Thanks for your suggestion.
Comment 61•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/4db00335d980
Keywords: checkin-needed
Comment 62•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4db00335d980
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Comment 63•9 years ago
|
||
Please request b2g34 and b2g37 approval on this patch when you get a chance.
Flags: needinfo?(amchung)
Target Milestone: 2.2 S9 (3apr) → 2.2 S13 (29may)
Comment 64•9 years ago
|
||
This issue is verified fixed on the latest 3.0 Nightly Flame build. Actual Results: The wifi network connects as expected and is usable without restarting the phone. Environmental Variables: Device: Flame 3.0 BuildID: 20150526010203 Gaia: 7cd4130d4f988562a77d126860408ada65bb95ef Gecko: 43f2f0c506ea Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67 Version: 41.0a1 (3.0) Firmware Version: v18D-1 User Agent: Mozilla/5.0 (Mobile; rv:41.0) Gecko/41.0 Firefox/41.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+][failed-verification] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Comment 65•9 years ago
|
||
Henry, can you help with the approval requests please? Comment 26 has been pending for 2 weeks now.
Flags: needinfo?(hchang)
Comment 66•9 years ago
|
||
Comment on attachment 8605028 [details] [diff] [review] Bug1123680.patch NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): Wifi User impact if declined: Unable to connect to wifi after forget Testing completed: Yes Risk to taking this patch (and alternatives if risky): No String or UUID changes made by this patch: No
Flags: needinfo?(hchang)
Attachment #8605028 -
Flags: approval-mozilla-b2g37?
Attachment #8605028 -
Flags: approval-mozilla-b2g34?
Comment 67•9 years ago
|
||
(In reply to Henry Chang [:henry] from comment #66) > Comment on attachment 8605028 [details] [diff] [review] > Bug1123680.patch > > NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to > better understand the B2G approval process and landings. > > [Approval Request Comment] > Bug caused by (feature/regressing bug #): Wifi > User impact if declined: Unable to connect to wifi after forget > Testing completed: Yes > Risk to taking this patch (and alternatives if risky): No > String or UUID changes made by this patch: No Hi Amy, Hi Henry, Do we need UT here? Thanks!
Flags: needinfo?(hchang)
Comment 68•9 years ago
|
||
(In reply to Josh Cheng [:josh] from comment #67) > (In reply to Henry Chang [:henry] from comment #66) > > Comment on attachment 8605028 [details] [diff] [review] > > Bug1123680.patch > > > > NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to > > better understand the B2G approval process and landings. > > > > [Approval Request Comment] > > Bug caused by (feature/regressing bug #): Wifi > > User impact if declined: Unable to connect to wifi after forget > > Testing completed: Yes > > Risk to taking this patch (and alternatives if risky): No > > String or UUID changes made by this patch: No > > Hi Amy, Hi Henry, > Do we need UT here? > Thanks! We don't have a |forget| unit test currently. However, |forget| is used several times in other integration test. I do think we need a |forget| unit test in the long run but not for this bug unless we can reproduce this issue on emulator which is the testbed that we run our wifi marionette test cases. Thanks!
Flags: needinfo?(hchang)
Updated•9 years ago
|
Attachment #8605028 -
Flags: approval-mozilla-b2g37?
Attachment #8605028 -
Flags: approval-mozilla-b2g37+
Attachment #8605028 -
Flags: approval-mozilla-b2g34?
Attachment #8605028 -
Flags: approval-mozilla-b2g34+
Comment 69•9 years ago
|
||
(In reply to Henry Chang [:henry] from comment #68) > We don't have a |forget| unit test currently. However, |forget| > is used several times in other integration test. > > I do think we need a |forget| unit test in the long run but not > for this bug unless we can reproduce this issue on emulator which > is the testbed that we run our wifi marionette test cases. > > Thanks! Thanks Henry!
Comment 72•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/0ebea88c344d
status-b2g-v2.1S:
--- → fixed
Comment 73•9 years ago
|
||
This Problem is verified as "pass" on latest build of Flame 2.1&2.2. My testing steps are: 1. Launch Settings -> Wi-Fi-> connect to the any wifi AP. 2. Tap this connected wifi AP and forget it. 3. Go back to wifi setting. 4. Tap this wifi AP and connect again to the network immediately. Actual Results: The wifi network connects as expected and it is usable without restarting the phone. See attachment: Verify_pass_2.1&2.2.3gp Flame 2.2 reproduce rate: 0/10 Flame 2.1 reproduce rate: 0/10 Device information: Flame2.1 (Pass) Build ID 20150617001205 Gaia Revision f8b848c82d1ed589f7a1eb5cc099830c867ff1d4 Gaia Date 2015-06-08 09:48:23 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/0ebea88c344d Gecko Version 34.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150617.035603 Firmware Date Wed Jun 17 03:56:15 EDT 2015 Bootloader L1TC000118D0 Flame 2.2 (Pass) Build ID 20150617002504 Gaia Revision 3414b07dc489976bf510fd8042c0af3b1192c160 Gaia Date 2015-06-16 22:04:56 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/a2db74491088 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150617.040422 Firmware Date Wed Jun 17 04:04:34 EDT 2015 Bootloader L1TC000118D0 I will verify 2.1S when the build version is reached.
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][MGSEI-Triage+]
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(amchung)
You need to log in
before you can comment on or make changes to this bug.
Description
•