Repository navigation
fix(ui): Consider typing and scrolling as activity for auto logout - #65589
Conversation
With `auto_logout` enabled, only mouse movements and touch inputs were considered an activity. This means that people were logged out after `session_lifetime` if they only type or scroll without moving the mouse itself. With this commit, key presses, text inputs (`beforeinput`, so dictation solutions do work as well) and mouse `wheel` inputs (instead of `scroll` to prevent scripts keeping the session active by e.g. simply scrolling to a new message) count as activity, too. Additionally, the last activity is shared with other tabs at most once per logout check interval now, and a failing storage write no longer throws. It also document and test that apps report activity through a storage event in the same window, which richdocuments relies on. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: David Dreschner <david.dreschner@nextcloud.com>
DerDreschner
added
bug
3. to review
|
/backport to stable35 |
|
/backport to stable34 |
|
/backport to stable33 |
There was a problem hiding this comment.
🟢 Approval recommended
Static review found no blocking issues, and focused regression tests cover the changes, though they were not executed during review.
0 open findings
What changed in this PR
Improves auto-logout activity detection so keyboard input and wheel scrolling keep sessions active.
Changes:
- Expands activity listeners and captures events stopped by editors.
- Limits cross-tab storage writes and handles write failures.
- Adds regression tests for activity detection, logout timing, and storage behavior.
| File | Description |
|---|---|
| core/src/tests/OC/session-heartbeat.spec.ts | Adds auto-logout regression tests and listener cleanup. |
| core/src/session-heartbeat.ts | Expands activity detection and throttles storage writes safely. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Capture phase, as editors may stop the propagation of input events | ||
| window.addEventListener('mousemove', markActive, { capture: true }) | ||
| // Touch, pen and clicks, including clicks by assistive technology that do not move the mouse | ||
| window.addEventListener('pointerdown', markActive, { capture: true }) | ||
| window.addEventListener('keydown', markActive, { capture: true }) | ||
| // Text insertion without key events, e.g. dictation | ||
| window.addEventListener('beforeinput', markActive, { capture: true }) | ||
| // Wheel instead of scroll events, as scripts fire scroll events too | ||
| window.addEventListener('wheel', markActive, { capture: true, passive: true }) |
There was a problem hiding this comment.
I am unsure if this is not too much of events, did you do a performance analysis?
If there is a bad performance impact here, we might need to trim this down to
- mousemove
- touchstart (mobile)
- keydown (typing or navigating with keyboard)
- wheel (scrolling long documents)
There was a problem hiding this comment.
Didn’t profile, but looking at the code this should actually be cheaper than before. On master every mousemove wrote to localStorage. Now each event only updates a timestamp, and localStorage gets written at most once per second.
There was a problem hiding this comment.
@pringelmann That's correct. But this code is only active when auto_logout is on, which is important to remember.
@susnux I did some testing with Claude after your suggestion and uses about 10 times less CPU resources when moving the mouse. Full report:
| Firefox master | Firefox PR | Firefox suggestion | Chrome master | Chrome PR | Chrome suggestion | |
|---|---|---|---|---|---|---|
| Initialization, once per page load (µs) | 23.3 | 26.7 | 26.7 | 81.3 | 88.0 | 83.0 |
| Page load, median (DOMContentLoaded / file list visible, ms) | 1116 / 1550 | 1110 / 1546 | 1085 / 1515 | 1244 / 1498 | 1213 / 1484 | 1224 / 1506 |
| Each storage event in another tab (µs) | 2.18 | 2.18 | 2.18 | 1.19 | 1.19 | 1.19 |
| Moving the mouse (60 mousemove/s) | 193 ± 51 + 392 = 585 (60/s) | 41 ± 26 + 6 = 48 (1/s) | 41 ± 26 + 6 = 48 (1/s) | 312 ± 10 + 214 = 527 (60/s) | 42 ± 11 + 4 = 46 (1/s) | 36 ± 23 + 4 = 39 (1/s) |
| Scrolling with wheel or trackpad (60 wheel/s) | 0 ± 1 + 0 = 0 ✗ (0/s) | 34 ± 1 + 6 = 40 (1/s) | 34 ± 2 + 6 = 40 (1/s) | -1 ± 0 + 0 = -1 ✗ (0/s) | 41 ± 2 + 4 = 44 (1/s) | 41 ± 1 + 4 = 44 (1/s) |
| Typing (6 keydown + 6 beforeinput/s) | 3 ± 4 + 0 = 3 ✗ (0/s) | 12 ± 10 + 6 = 18 (1/s) | 11 ± 16 + 6 = 18 (1/s) | -6 ± 4 + 0 = -6 ✗ (0/s) | 31 ± 10 + 4 = 35 (1/s) | 24 ± 7 + 4 = 27 (1/s) |
| Touch scrolling (1 swipe/s) | 6 ± 1 + 6 = 12 (1/s) | 6 ± 1 + 6 = 12 (1/s) | 6 ± 1 + 6 = 12 (1/s) | 6 ± 0 + 4 = 9 (1/s) | 8 ± 1 + 4 = 11 (1/s) | 8 ± 1 + 4 = 11 (1/s) |
| Mouse + scrolling + typing at once | 180 ± 24 + 392 = 572 (60/s) | 48 ± 21 + 6 = 54 (1/s) | 54 ± 29 + 6 = 60 (1/s) | 289 ± 30 + 214 = 504 (60/s) | 140 ± 35 + 4 = 144 (1/s) | 138 ± 48 + 4 = 142 (1/s) |
How to read a use-case cell: CPU time in µs per second of that activity, as this tab ± noise + 3 other open Nextcloud tabs = total, with localStorage writes per second (each one reaches every other tab) in brackets. ✗ means master doesn't detect this activity, so the user gets logged out. Negative values are noise around zero. One second of one CPU core is 1,000,000 µs.
|
This pull request has been automatically locked because it has been closed for more than 7 days. As we do not monitor merged or closed pull requests, we are locking this pull request to prevent further comments from getting lost in the noise. |
Summary
When using
auto_logout, only mouse movements and touch were considered an activity. In case thesession_lifetimeis set to something relatively short, users can be logged out if they only type for a pretty long time or only use the keyboard to navigate.This is changed with this PR, which now considers the following events an activity:
mousemovepointerdownkeydownbeforeinputwheelThis cover almost every use-case. Known gaps and exceptions that Claude identified:
richdocumentsand uses a storage event in the same window, which is being documented and tested with this PR.Additionally, the last activity is shared with other tabs at most once per logout check interval now, and a failing storage write no longer throws.
Checklist
3. to review, feature component)stable32)AI (if applicable)