Closed Bug 934172 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox OS Graveyard :: Wifi, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:koi+, firefox26 wontfix, firefox27 wontfix, firefox28 verified, b2g-v1.2 verified)

VERIFIED FIXED
blocking-b2g koi+
Tracking Status
firefox26 --- wontfix
firefox27 --- wontfix
firefox28 --- verified
b2g-v1.2 --- verified

People

(Reporter: whsu, Assigned: chucklee)

Details

(Keywords: regression, Whiteboard: burirun3)

Attachments

(1 file, 3 obsolete files)

* 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
blocking-b2g: --- → koi?
Whiteboard: BuriRun3
Whiteboard: BuriRun3 → burirun3
Summary: [WIFI][V1.2][BuriRun3] Cannot pop-up a notification when detect a available network → [Captive Portal][V1.2][BuriRun3] Cannot pop-up a notification when detect an available WIFI
Broken feature and regression from 1.0.1
blocking-b2g: koi? → koi+
Keywords: regression
Ken can you please help with an assignee here ?
Flags: needinfo?(kchang)
Chuck, we need your help for this bug.
Assignee: nobody → chulee
Flags: needinfo?(kchang)
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?
Flags: needinfo?(whsu)
Maybe we can improve detection rate by retry when error and timeout?
Attachment #832762 - Flags: feedback?(schien)
Attachment #832762 - Attachment description: 0001. Retry on error and timeout. → WIP - Retry on error and timeout.
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.
Attachment #832762 - Flags: feedback?(schien) → feedback+
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.
Flags: needinfo?(whsu)
@whsu, which location you perform the test? We can verify if extending retry mechanism really helps the test environment you encountered.
Flags: needinfo?(whsu)
I did the test at 7-11 of Xinyi Sports Center.
Thanks! :)
Flags: needinfo?(whsu)
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.
Attached patch Retry on error and timeout. V1 (obsolete) — Splinter Review
1. Set default retry parameter.
2. Address comment 6.
Attachment #832762 - Attachment is obsolete: true
Attachment #8333652 - Flags: review?(schien)
Attached patch Retry on error and timeout. V2 (obsolete) — Splinter Review
Timeout is in ms.
Attachment #8333652 - Attachment is obsolete: true
Attachment #8333652 - Flags: review?(schien)
Attachment #8333654 - Flags: review?(schien)
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.
Attachment #8333654 - Flags: review?(schien) → review+
Address reviewer's comments.
Attachment #8333654 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/9cad550b8c57
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.