New issue
Advanced search Search tips

Issue 838974 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

CaptivePortalWindowProxy unused?

Project Member Reported by est...@chromium.org, May 2 2018

Issue description

Is CaptivePortalWindowProxy in use?

This is a dialog that appears to be used from the Chrome OS oobe, however I can find no way to trigger it without adding debug code. It is NOT the dialog that appears when you try to connect to a portaled wifi network, get a notification, and click on the notification[1]. With debug code enabled, it looks like the attached screenshot. 

In Mash, it straight up crashes because of the rather odd widget initialization code[2] which sets child to true and doesn't provide a non-client frame view, thus falling back to ViewsDelegate::CreateDefaultNonClientFrameView which relies on ash::Shell. The crash should be fairly easy to fix but I'm not sure if the code is actually used. It is exercised in some browser tests though:
-CaptivePortalWindowCtorDtorTest.*
-CaptivePortalWindowTest.*

[1] that is a singleton tab. 
 https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/network/netwok_portal_notification_controller.cc?rcl=a5f079381aa96a95dc5e60c064105849b5b7dc58&l=194

[2] https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/ui/captive_portal_window_proxy.cc?rcl=a5f079381aa96a95dc5e60c064105849b5b7dc58&l=19
 
DG7vYymT50b.png
35.8 KB View Download
The reason that dialog looks so hokey is that it uses SimpleWebViewDialog (in fact it's the only thing that uses that class), which has bespoke code for the toolbar and fails to respond to changes in the native theme (hence kPlaceholderColor, i.e. red, being used). Best case scenario, that's another big chunk of code we can delete.
Cc: alemate@chromium.org
+alemate@ who might know more.

Scanning through the code, it appears to be called via ErrorScreen::ShowCaptivePortal(), which appears to be triggered in a number of places during login and enrollment.

Yes, I saw call sites, but I couldn't figure out how to ever trigger it except via debug code, and the appearance is so janky I'd hope we'd have seen bug reports if it ever showed up in practice.
I tried to look back into the history, but it started in 2012 with fully-native UI. We have since passed through the few large rebuilds, so it's difficult to find when we lost the way to start it.


Let me actually dig a bit more, may be I'll find what has changed ;)
It looks like that it is still possible to trigger this.

It is invoked from https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/screen_error_message.js?l=171 , which is is shown in https://cs.chromium.org/chromium/src/chrome/browser/resources/chromeos/login/screen_error_message.html?l=73 , and it sounds like this link is visible to user if portal was detected on OOBE Update screen, signin screen (probably cannot happen), supervised (I think it was creation of supervised users, which is also not possible), kiosk-mode (on kiosk session launch, I don't know whether it actually can happen, because we should not be locked on network error), and 
 auto-enrollment-error .


So yes, I believe it is still possible to get this screen. We probably need to completely replace it with references to the new network portal sign-in dialog.
I can see references in code, but I still haven't heard of anyone being able to trigger it in practice. For example when I do an OOBE and choose a portaled wifi network, it shows the other dialog (footnote [1] in original report).
I believe you need your network to become portal network while updating.
Labels: Hotlist-OOBE-polish
Status: Available (was: Untriaged)
Labels: Enterprise-Triaged

Sign in to add a comment