Accessible captive portal when settings are open

[guyonvarch]

Previously, when the settings modal was open, when a captive portal was detected, we showed a header with a button, but it could not be interacted with, because the modal didn’t allow to access the parent window.

Setup the settings modal to be modeless, so that the button to open the captive portal is still accessible.

Also better represent the dialog state with an enumeration.

See https://trello.com/c/wcIrvEau/1445-captive-portal-prompt-pops-up-beneath-controller-modal

Testing

• Test dialogs in VM or live.

Checklist

• Changelog updated
• Code documented
• [ ] User manual updated

[guyonvarch]

Dialogs working as expected in VM.

I used the following diff to trigger the captive portal:

diff --git a/kiosk/kiosk_browser/captive_portal.py b/kiosk/kiosk_browser/captive_portal.py
index bdc0759..299f836 100644
--- a/kiosk/kiosk_browser/captive_portal.py
+++ b/kiosk/kiosk_browser/captive_portal.py
@@ -53,9 +53,10 @@ def is_likely_replaced_page(status_code):
 class CaptivePortal():
 
     def __init__(self, get_current_proxy, show_captive_portal_message):
-        self._status = Status.DIRECT_DISCONNECTED
+        self._status = Status.DIRECT_CAPTIVE
         self._get_current_proxy = get_current_proxy
         self.show_captive_portal_message = show_captive_portal_message
+        self.show_captive_portal_message('https://captive.dividat.com')
 
     def start_monitoring_daemon(self):
         thread = threading.Thread(target=self._check, args=[])
@@ -64,36 +65,8 @@ class CaptivePortal():
 
     def _check(self):
         while True:
-            proxy = self._get_current_proxy()
-
-            if proxy is not None:
-                self._status = Status.PROXY
-            else:
-                try:
-                    r = requests.get(check_connection_url, allow_redirects = False)
-
-                    if r.status_code == HTTPStatus.OK and 'Open Sesame' in r.text:
-                        self._status = Status.DIRECT_CONNECTED
-
-                    elif is_redirect(r.status_code):
-                        self._status = Status.DIRECT_CAPTIVE
-                        self.show_captive_portal_message(r.headers['Location'])
-
-                    elif is_likely_replaced_page(r.status_code):
-                        self._status = Status.DIRECT_CAPTIVE
-                        self.show_captive_portal_message(check_connection_url)
-
-                    else:
-                        self._status = Status.DIRECT_DISCONNECTED
-
-                except requests.exceptions.RequestException as e:
-                    self._status = Status.DIRECT_DISCONNECTED
-                    logging.error('Connection request exception: ' + str(e))
-
-                except Exception as e:
-                    self._status = Status.DIRECT_DISCONNECTED
-                    logging.error('Connection exception: ' + str(e))
-
+            self._status = Status.DIRECT_CAPTIVE
+            self.show_captive_portal_message('https://captive.dividat.com')
             sleep(self._status)
 
 def open_message(on_open):

I don’t currently understand why I need to force the initial state to be captive to get the message. This is not required when running the kiosk as a standalone.

[guyonvarch]

When the captive portal is open, pressing CTRL+SHIFT+F12, I wondered if it should directly close the captive portal and open the settings. We could do that, but for the moment, I selected an easier solution to implement, that is only closing the captive portal when pressing CTRL+SHIFT+F12.

[knuton]

Nice, I tested with a real captive portal and it works well. 👌

Detected one unused code and suggest considering slightly different data structure.

[knuton]

I noticed that the modeless dialog does not receive focus automatically, so Escape to close and tabbing inside the controller interface no longer works until clicking into the dialog.

We should be able to give focus explicitly as in this example:

dialog.raise()
dialog.activateWindow()

[guyonvarch]

I noticed that the modeless dialog does not receive focus automatically, so Escape to close and tabbing inside the controller interface no longer works until clicking into the dialog.

Did you notice this on the VM, live, or on the kiosk directly? I have the same problem using the kiosk standalone, but no problem using the VM.

Using raise_ (not sure if it’s equivalent to raise, but I couldn’t use raise, it’s a keyword in Python) and activateWindow didn’t change for me. Maybe it’s working on X.

[guyonvarch]

I have the same problem using the kiosk standalone, but no problem using the VM.

Well, I need to click on the modal to be able to press escape using live. I’ll have another look. Another solution is to remove using escape, or allow to close pressing escape if the dialog does not have focus.

[knuton]

Did you notice this on the VM, live, or on the kiosk directly? I have the same problem using the kiosk standalone, but no problem using the VM.

In a live system booted from USB on a test machine, so pretty much the real deal.

Another solution is to remove using escape, or allow to close pressing escape if the dialog does not have focus.

This does not resolve the more serious problem which is loss of tabbability. We need tab navigation so the controller may be used without a mouse.

[knuton]

Works fine for me on a live system with this modification (moving show to after setting the property is unrelated, may or may not be a good idea to avoid race conditions):

diff --git a/kiosk/kiosk_browser/main_widget.py b/kiosk/kiosk_browser/main_widget.py
index 366374d..d698e26 100644
--- a/kiosk/kiosk_browser/main_widget.py
+++ b/kiosk/kiosk_browser/main_widget.py
@@ -82,10 +82,12 @@ class MainWidget(QtWidgets.QWidget):
                 additional_close_keys = [self._toggle_settings_key],
                 on_close = lambda: self._close_dialog()
             )
+        self._dialog = Settings(dialog)
         # Open modeless to allow accessing captive portal message banner
         # https://doc.qt.io/qtforpython-5/PySide2/QtWidgets/QDialog.html#modeless-dialogs
         dialog.show()
-        self._dialog = Settings(dialog)
+        dialog.raise_()
+        dialog.activateWindow()
 
     def _show_captive_portal(self):
         self._close_dialog(reload_browser_widget = False)
@@ -98,8 +100,10 @@ class MainWidget(QtWidgets.QWidget):
                 additional_close_keys = [self._toggle_settings_key],
                 on_close = lambda: self._close_dialog()
             )
-        dialog.show()
         self._dialog = CaptivePortal(dialog)
+        dialog.show()
+        dialog.raise_()
+        dialog.activateWindow()
 
     def _close_dialog(self, reload_browser_widget = True):
         match self._dialog:

Maybe this triumvirate of method calls, which we always want in this context, could be encapsulated in a function or via a specialized class that is used for both kinds of dialog.

[guyonvarch]

I tested in a live system, this works OK: 927acef

I’d still like to try a dialog less approach, which would potentially allow to tab between the settings and the button to open the captive portal.

[knuton]

Good call, the portal dialog can still be modal!

[knuton]

I am going to merge so we can move towards a release, but feel invited to continue iterating on it.