Skip to content

Conversation

@yfyf
Copy link
Collaborator

@yfyf yfyf commented Oct 29, 2025

...and also deal with a bunch orthogonal annoyances, including:

  • Make kiosk button styling uniform and focusing more visible
  • Prevent kiosk buttons from handling spatial navigation (needed to avoid wrapping)
  • Remove the multi-action dialog that is redundant now

As usual with Qt, this turned out to be quite elaborate and quite silly, but is the best I can come up with. It should be fairly robust to various re-arrangements and already handles situations like "double stacking" when in PlayOS settings (i.e. dialog) and captive portal message is visible.

I tried and/or considered several different approaches, but they seemed even messier:

  • "manual" focus in/out handling in BrowserWidget - this turned out to be a mixture of the current approach and extra manual handling, so is even harder to grok, leaks the logic across multiple widgets
  • "marking" widgets as "top" / "bottom" explicitly and having them override focusNextPrevChild behaviour to prevent wrapping - this seemed way more cumbersome and requires to track/propagate info about the "top" widget
  • translating arrow key presses into Tab / Shift+Tab - this avoids all the boiler plate code, but it means navigation is very unpredictable spatially

Remaining questions/issues, a bit orthogonal:

  • If wrapping is acceptable, then this can be greatly simplified, but I think wrapping sucks?
  • The button styling is a bit crude, the border is not very visible on a light background.
  • The focus-shift:exhausted events do not trigger on many pages I tried, making it impossible to focus on the close button in Network login (captive portals).
  • There are some issues with double-execution of focus-shift in some strange pages with sub-frames, we can look at this separately.

Checklist

  • Changelog updated
  • Code documented
  • User manual updated

yfyf added 5 commits October 27, 2025 16:47
This commit does several things:
- Introduce central arrow Up/Down arrow handling into MainWidget for
  spatial navigation
- Translate focus-shift:exhausted events into Qt key events
- Make kiosk button styling uniform and focusing more visible
- Prevent kiosk buttons from handling spatial navigation
- Remove the multi-action dialog that is redundant now
It would be possible to split this script into multiple more specific
ones, but 1) we use all of it on all pages unconditionally; 2) it would
require more complicated QWebChannel handling (e.g. via a CustomEvent)
for sharing among the sub-scripts, making the JS code more complex.
Currently it errors out with

        js: Uncaught ReferenceError: qt is not defined

when there are any sub-frames in the page.

Not sure what is the root cause/ and fix.

This implies that opaque mode will not work in sub-frames, however
sub-frames are anyway non-focusable with the remote control anyway, so
it does not matter.
@yfyf yfyf requested a review from knuton October 29, 2025 15:29
@yfyf yfyf added the reviewable Ready for initial or iterative review label Oct 29, 2025
yfyf added 2 commits October 30, 2025 14:41
This would lead to some None's along the way, causing a crash.

This seems to be purely synthetic / dev env related (e.g. while
navigating inside of Chrome DevTools).
After digging a bit deeper into QWebEngine's and Chromium userscript
guts, it's more clear what is going on:

- `window.qt.webChannelTransport` is the Chromium IPC transport and is
  created by QWebEngine itself when you call `setWebChannel`. It will be
  available only in the specified worldId (which is MainWorld by
  default) and ONLY in the top-level window (not on iframes). The latter
  seems to be a strange limitation in the implementation.
- "structured clone" is not allowed between worlds, which is why scripts
  running in different worlds can see the dispatched events, but cannot
  read `event.detail` if it is an object. `event.detail:
  JSON.stringify(...)` is fine though
- If two (user)scripts expect to communicate, they have the following
  options:

        1. Run in the same worldId
        2. Use window.dispatchEvent, but with event.detail of type
           string (avoids structured clone)
        3. Use runtime.sendMessage
        4. Use window.postMessage

- The above also applies if a user script wants exchange events with a
  script running on the page (e.g. focus-shift embedded in Play, or
  kiosk dispatching events to Play)!

These points imply that we either run FocusShiftBridge (and hence
FocusShiftScript) on MainWorld or... if we want to run
in a more isolated setup (e.g. in ApplicationWorld), then we have the
following options:

a. JSON.stringify all `event.details`
b. Create a separate "Proxy" script that runs on MainWorld and
   handles event forwarding/translation between the page and the
   user scripts. This can be done in several ways (again, via
   window.postMessage, runtime.sendMessage, or even regular DOM
   events, but with detail as string).

Since it does not seem like we need the extra isolation currently (e.g.
i.e. don't expose anything dangerous via the QWebChannel), MainWorld is
probably good enough.
@yfyf
Copy link
Collaborator Author

yfyf commented Oct 30, 2025

Discovered one minor issue/quirk: if you focus back into the web view and the focus is restored on a text/numeric input field, then it will not be possible to open the virtual keyboard without focus-shifting inside the web view. There are no events emitted by Qt, so it's not a bug in our code. It is very similar to the "refocus" Qt bug reported a while ago.

We can try to handle it with various hacks, but since this currently can only happen on pages where the top-most element is a text/numeric input (i.e. not currently on Play or PlayOS, might be some captive portals) and since it is recoverable (just move focus around), I would say it can be left as is.

yfyf added 2 commits October 30, 2025 16:43
Good hygiene, also avoids strange interactions with some pages that
somehow re-eval user-scripts.
@yfyf
Copy link
Collaborator Author

yfyf commented Oct 30, 2025

@knuton discovered a case of focus-shift:exhausted not firing even on our own pages, steps to reproduce:

  • Open PlayOS Settings>Network
  • Open a Wired network
  • It is now not possible to navigate to the Close button with arrow keys (both from the nav menu on the left and from the Wired sub-page)

Note that it seems to work fine in all other pages, so this is something quite specific.

@knuton
Copy link
Member

knuton commented Oct 31, 2025

Looks good, yet to perform practical tests.

@knuton
Copy link
Member

knuton commented Nov 4, 2025

Works nicely overall.

When I open the kiosk on my laptop with check URL for dev-tools/captive-portal.py configured, the captive portal nagbar comes up immediately but I can not initially move focus to it with the arrow keys (tabbing works). When I focus another window and then activate the kiosk again, the captive portal button is focused. This might just be because the loading page does not have focus-shift?

(I have not tested on a physical device yet, will do.)

image

Edit: When I made the screenshot, focus seems to have moved to the button. 🙃 Anyway, this is the screen I am talking about.

@yfyf
Copy link
Collaborator Author

yfyf commented Nov 4, 2025

the captive portal nagbar comes up immediately but I can not initially move focus to it with the arrow keys (tabbing works).

Good catch.

This might just be because the loading page does not have focus-shift?

Yes, most likely. I will do some testing with an empty page and an infinitely loading page to check what can be done.

When I focus another window and then activate the kiosk again, the captive portal button is focused.

Classic scenario 💀

@knuton knuton added changes suggested Asking for changes before next round of reviewing and removed reviewable Ready for initial or iterative review labels Nov 4, 2025
Ensures that the currently visible view has focus and key events
are properly propagated.

This fixes situations where network errors or pages that are stuck in
loading prevent the user from being able to shift focus to the other Qt
widgets.
@yfyf
Copy link
Collaborator Author

yfyf commented Nov 4, 2025

I can not initially move focus to it with the arrow keys

Should be fixed in 750ca7d. Tested with both an infinitely loading page and a network error scenario.

@yfyf yfyf added reviewable Ready for initial or iterative review and removed changes suggested Asking for changes before next round of reviewing labels Nov 4, 2025
@knuton knuton merged commit 494abab into dividat:main Nov 4, 2025
25 checks passed
@knuton knuton removed the reviewable Ready for initial or iterative review label Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants