Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add key combo for hard refresh in kiosk #147

Merged
merged 3 commits into from
Jan 4, 2024

Conversation

knuton
Copy link
Member

@knuton knuton commented Dec 19, 2023

Add a conventional key combination to perform a hard refresh, useful for recovering if corrupted assets end up being cached. By using the Ctrl-Shift-R combination common in browsers for Windows and Linux, this may be discoverable by a savvy admin without the need to be told explicitly.

Testing

I started the kiosk with a debug port, opened chrome://inspect and checked that performing a hard refresh causes files to be downloaded again. This was made easier by simulating a slow network in the debug session DevTools.

Checklist

  • Changelog updated
  • Code documented
  • User manual updated

@knuton knuton added the reviewable Ready for initial or iterative review label Dec 19, 2023
Copy link
Contributor

@stoeffel stoeffel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good to me, I still didn't get around to setup my intel mac, so I couldn't test it.

Add a conventional key combination to perform a hard refresh, useful for
recovering if corrupted assets end up being cached. By using the
Ctrl-Shift-R combination common in browsers for Windows and Linux, this
may be discoverable by a savvy admin without the need to be told
explicitly.
Copy link
Member

@guyonvarch guyonvarch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clearing the cache as expected!

Using the kiosk, and pressing Ctrl+Shift+R, I get an infinite loader, and I had to press Ctrl+R to load the page:

2023-12-22_15-38-28.mp4

Did you get this behavior on your tests?

kiosk/kiosk_browser/browser_widget.py Outdated Show resolved Hide resolved
kiosk/kiosk_browser/browser_widget.py Outdated Show resolved Hide resolved
@guyonvarch guyonvarch added changes suggested Asking for changes before next round of reviewing and removed reviewable Ready for initial or iterative review labels Dec 22, 2023
@knuton
Copy link
Member Author

knuton commented Dec 22, 2023

Using the kiosk, and pressing Ctrl+Shift+R, I get an infinite loader, and I had to press Ctrl+R to load the page:

I saw something similar, though I thought it only happened when I was using the debug connection and concluded it was due to that.

I can recheck whether it also happens when DevTools are not connected.

@guyonvarch
Copy link
Member

I can recheck whether it also happens when DevTools are not connected.

I just checked and got the same problem without dev tools being open.

This works around the race condition described in
https://bugreports.qt.io/browse/QTBUG-111541 by waiting an instant after
triggering the clearing of profile cache. If a load is triggered while
the clearing is not finished, it can cause the load to hang. This
can be recovered from by triggering a simple reload, but is better to
avoid.

A signal for observing the progress of cache clearing should be
available for future versions of Qt 6:
qt/qtwebengine@3743002

I considered adding a more involved mechanism to detect stalled loads
and automatically trigger a new load, but concluded that a sleep should
be overall simpler and easy enough to understand. As a relatively short
sleep is chosen to avoid user confusion, this may still fail, but a
hanging load is relatively harmless and should be recoverable by the
user.
@knuton
Copy link
Member Author

knuton commented Jan 3, 2024

Using the kiosk, and pressing Ctrl+Shift+R, I get an infinite loader, and I had to press Ctrl+R to load the page:

Thanks for catching this! Workaround and explanation in 6cc13db.

@knuton knuton added reviewable Ready for initial or iterative review and removed changes suggested Asking for changes before next round of reviewing labels Jan 3, 2024
Copy link
Member

@guyonvarch guyonvarch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No infinite loading anymore on the kiosk running directly, and on the VM. 👌

@guyonvarch guyonvarch merged commit 28381e1 into dividat:main Jan 4, 2024
1 check passed
@guyonvarch guyonvarch removed the reviewable Ready for initial or iterative review label Jan 4, 2024
@knuton knuton deleted the kiosk-hard-reload branch January 4, 2024 10:40
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.

3 participants