934172 - [Captive Portal][V1.2][BuriRun3] Cannot pop-up a notification when detect an available WIFI

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[William Hsu [:whsu]]

* Description:
  This problem happens on v1.2 Buri build.
  As current specification, if FxOS detects an available WIFI, the FxOS will pop-up a notification.
  The message show as below.
  - "The network XXX was found. Join network?" 

* Reproduction steps:
  1. Open the Settings app, enable Wi-Fi.
  2. Click the captive portal Wi-Fi.
  3. Long press the Home button, close the Browser app.
  4. Open the Settings app, and disable Wi-Fi.
  5. Enable Wi-Fi, then press Home button.

* Expected result:
  The notification "The network XXX was found. Join network?" should be presented.

* Actual result:
  No notification pop up

* Reproduction build:(V1.2)
 - Gaia:     a788ea1a3e04716938bd41a559c36a831cf1190b
 - Gecko:    http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/4fbe1b01ed63
 - BuildID   20131030004003
 - Version   26.0a2

Comment 1

[Preeti Raghunath(:Preeti)]

Broken feature and regression from 1.0.1

Comment 2

[bhavana bajaj [:bajaj]]

Ken can you please help with an assignee here ?

Comment 3

[Ken Chang[:kenkai]]

Chuck, we need your help for this bug.

Comment 4

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

I observes that in such condition, detection fail is caused by connection lost or connection timeout.
After a little chat with schien, he said that captive portal detection is not 100% accurate, and wifi connection speed and stability affects the detection a lot.
Also I found some network with captive portal doesn't require login every time. For such case, there's no require to show up the browser if user doesn't have to login again.

Do you have detail on your test environment and reproduce rate?

Comment 5

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: WIP - Retry on error and timeout.

Maybe we can improve detection rate by retry when error and timeout?

Comment 6

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 832762 [details] [diff] [review]
WIP - Retry on error and timeout.

Review of attachment 832762 [details] [diff] [review]:
-----------------------------------------------------------------

f+ with the idea of retry on more scenarios, but I'm still curious about the test environment and reproduce rate that @whsu is facing.

::: toolkit/components/captivedetect/captivedetect.js
@@ +299,5 @@
>      let urlFetcher = new URLFetcher(this._canonicalSiteURL, this._maxWaitingTime);
>  
>      let requestDone = this.executeCallback.bind(this, true);
> +    let retryDetection = this._retryDetection.bind(this);
> +    let errorRetry = function () {

I think you can aggregate this local function with _retryDetection() into a class function called _mayRetry(). Therefore, you can have a shorter code in this function.

Comment 7

[William Hsu [:whsu]]

Hi, Chuck,

Which detail information do you need?
Do you mean the network SSID or ?

I used "TPE-FREE" to do the test and reproduction rate is 100%.
Thanks.

Comment 8

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

@whsu, which location you perform the test? We can verify if extending retry mechanism really helps the test environment you encountered.

Comment 9

[William Hsu [:whsu]]

I did the test at 7-11 of Xinyi Sports Center.
Thanks! :)

Comment 10

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

I tests about 10 times on Unagi(because I don't have buri), and all tests are success.
But the log shows that the timeout retry only recovers one detection, the other nine detections succeeded through original path.

So retry do help, but the failure rate of first try doesn't be that high in that environment.

Comment 11

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Retry on error and timeout. V1

1. Set default retry parameter.
2. Address comment 6.

Comment 12

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Retry on error and timeout. V2

Timeout is in ms.

Comment 13

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8333654 [details] [diff] [review]
Retry on error and timeout. V2

Review of attachment 8333654 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with following comment addressed.

::: b2g/app/b2g.js
@@ +773,5 @@
>  pref("captivedetect.canonicalURL", "http://detectportal.firefox.com/success.txt");
>  pref("captivedetect.canonicalContent", "success\n");
> +pref('captivedetect.maxWaitingTime', 3000);
> +pref('captivedetect.pollingTime', 3000);
> +pref('captivedetect.maxRetryCount', 5);

I think you can change the global default value.
http://dxr.mozilla.org/mozilla-central/source/modules/libpref/src/init/all.js#4519

::: toolkit/components/captivedetect/captivedetect.js
@@ +306,3 @@
>      urlFetcher.onsuccess = function (content) {
>        if (self.validateContent(content)) {
>          requestDone();

The variable |requestDone| is useless now. You can expand this line to |self.executeCallback(true);| and remove the let declaration.

@@ +319,3 @@
>        }
> +
> +      mayRetry();

I suggest to use |else| statement instead of early return. e.g.
if (...) {
  self._startLogin();
} else {
  self._mayRetry();
}

@@ +340,5 @@
> +    if (this._runningRequest.retryCount++ < this._maxRetryCount) {
> +      debug('retry-Detection: ' + this._runningRequest.retryCount + '/' + this._maxRetryCount);
> +      this._startDetection();
> +      return;
> +    }

Also use |else| statement here.

Comment 14

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Retry on error and timeout. V3

Address reviewer's comments.

Comment 15

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try : https://tbpl.mozilla.org/?tree=Try&rev=b3a3a58e37a4

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/9cad550b8c57

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9cad550b8c57

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/b06cc697da06

Comment 19

[Josh Schmitt [Joshs]]

The issue does not reproduce. The notifications for connecting to a network now shows in the latest Buri 1.2 and 1.3

Buri 1.2
Environmental Variables:
Device: Buri v1.2 COM RIL
BuildID: 20131206004002
Gaia: d48df33cbdae2063f12b09e3dca07ff7a2f2a3cc
Gecko: fd0302696c57
Version: 26.0
Firmware Version: v1.2_20131115

Buri 1.3
1.3 Environmental Variables:
Device: Buri v1.3 MOZ RIL
BuildID: 20131205040201
Gaia: 1dd0e5c644b4c677a4e8fa02e50d52136db489d9
Gecko: 725c36b5de1a
Version: 28.0a1
Firmware Version: V1.2_20131115