1320087 - [Captive Portal] - Captive Portal Notification bar and Show Login button enabled although no CP wifi is available anymore

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Adrian Florinescu [:aflorinescu]]

[Enviroment:}
OSes:
- Ubuntu 16.04 x64
- Mac  OSX 10.12
- Windows 10 x64

53.0a1 Build ID  20161121074938   

[Description]: After you enter into a successful captive portal detection phase and you disconnect the wireless the Notification bar and Show Login button are still shown.


[Steps]:
    1. Disconnect all network connections.
    2. Enable wifi and attempt connecting to a Captive portal capable . (do not finalize the connection by actually logging in)
    3. Start FF.
    4. Wait for the Captive Portal Detection to be initialized and successful.
    5. Once the CP Notification bar and Show Login button are displayed in FF, disconnect wi-fi.
    
    
[Actual Result]:
        The Notification bar and Show login button are still available in FF. 
        Opening new tab will still display the CP notification bar.
        Pressing Show Login button will redirect you to the CP tab if opened or open a new CP tab if not, detectportal.firefox.com/success.txt
        
        
        Console log on a debug enabled build shows the following ongoing check:
           " -*- CaptivePortalDetector component: checking if public network is available after the login procedure
            -*- CaptivePortalDetector component: checking if public network is available after the login procedure
           ...."

[Expected Result]:
         Once the CP detection returns false, retry a few times and if the CP state is detected to be false, remove the Notification bar and Show Login button.

Comment 1

[Matthew N. [:MattN]]

[Tracking Requested - why for this release]: Showing an inaccurate notification bar to users

Valentin, Is this something you can fix and uplift to 52?

Comment 2

[Valentin Gosu [:valentin] (he/him)]

This is a really tricky case. At this point the captive portal detection has no information regarding the state of the link. All it knows is if it can reach the 'canonical' endpoint or not. As it stands, it's quite difficult to distinguish between the next two scenarios:
CP detected - Wifi Disconnected - Can't reach endpoint
CP detected - Wifi times out - Can't reach endpoint

I'll try to reproduce the issue, and maybe find some solution for an improved detection of this case.

Comment 3

[Julien Cristau [:jcristau]]

Tracking this captive portal related issue for 52

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: Make the captive portal report UNKNOWN when the wifi is down

* Start/Stop the captive portal service in nsIOService::SetConnectivityInternal
* Set the captive portal state to UNKNOWN when stopping it.

MozReview-Commit-ID: 5dude1F4lNb

Comment 5

[Julien Cristau [:jcristau]]

Valentin what's the status of this patch?

Comment 6

[Valentin Gosu [:valentin] (he/him)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e12b1fe418a2

Comment 7

[Adrian Florinescu [:aflorinescu]]

(In reply to Valentin Gosu [:valentin] from comment #6)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=e12b1fe418a2

Redid the test with the above try-build. Comment 0 steps appear to be fixed as far as I can tell. (I will redo this test once it lands on central).

Logs after the disconnect of the wi-fi, after which the Captive portal notification bar is immediately removed:
*- CaptivePortalDetector component: sendEvent "true"
-*- CaptivePortalDetector component: abort for captive-portal-inteface
-*- CaptivePortalDetector component: detach HttpObserver for login activity
-*- CaptivePortalDetector component: sendEvent "{"type":"captive-portal-login-abort","id":"0"}"
-*- CaptivePortalDetector component: remove running request

Comment 8

[Valentin Gosu [:valentin] (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/eabf0117673e41abf14e13a46b5be5f9aadcc3a0
Bug 1320087 - Make the captive portal report UNKNOWN when the wifi is down r=bagder

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/eabf0117673e

Comment 10

[(inactive) Nihanth Subramanya [:nhnt11]]

Valentin, seems like this bug is good to go, can we request uplift to 52? :)

Comment 11

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8816629 [details] [diff] [review]
Make the captive portal report UNKNOWN when the wifi is down

Approval Request Comment
[Feature/Bug causing the regression]: Bug in the CP service detection.
[User impact if declined]: The UI will stay on screen after disconnecting from the network.
[Is this code covered by automated tests?]: No.
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: see comment 0
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: very few changes were made to ensure correctness.
[String changes made/needed]: none

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8816629 [details] [diff] [review]
Make the captive portal report UNKNOWN when the wifi is down

captive portal fix for aurora52

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b618acd89350

Comment 14

[Adrian Florinescu [:aflorinescu]]

verified as fixed on:

52.0a2 20170119004006
53.0a1 20170119030211

Comment 16

[Camelia Badau [:cbadau], Desktop QA]

Marking VERIFIED FIXED based on comment 14.