Skip to content

Safeguard lab answer saving against blank overwrites (Firefox)#285

Merged
italovalcy merged 3 commits into
mainfrom
enhance/safe-guard-save-answers
Jul 11, 2026
Merged

Safeguard lab answer saving against blank overwrites (Firefox)#285
italovalcy merged 3 commits into
mainfrom
enhance/safe-guard-save-answers

Conversation

@italovalcy

Copy link
Copy Markdown
Contributor

What

Adds defense-in-depth so a blank/partial form submission can never wipe a user's saved lab answers, and adds extra logging around the save path.

Why

Firefox users reported their lab answers being replaced by a dict of all answer names with empty-string values. Root cause: Firefox's built-in form-state restoration re-blanks the guide inputs after page load and dispatches synthetic change/input events. Those events hit the debounced auto-save, which serialized the now-blank form and POSTed {q1:"", q2:"", ...}, and the server overwrote the good answers.

Changes

Client (apps/templates/pages/lab_instance_view.html):

  • AsaveAnswers() bails out before POSTing when every field is blank, so an all-empty form can never overwrite stored answers.
  • B — sets autocomplete="off" on all #lab-guide inputs so Firefox stops restoring/re-blanking them after load (removing the source of the spurious change events).
  • Retains the existing answersLoaded guard that suppresses auto-save until the initial load settles.

Server (apps/api/routes.py):

  • Dsave_lab_answers now merges per-key into the stored answers instead of replacing them: an empty/None incoming value is skipped when a stored answer already exists, so even a partially blank payload cannot erase individual saved answers.

Also adds extra logging around the save-answers flow to aid future diagnosis.

Reviewer notes

  • The three layers are complementary: B stops the bad events, A stops the bad client POST, D stops the bad server write. Any one of them alone closes the reported wipe; together they're robust.
  • Trade-off of A: a user cannot auto-save an intentional "clear every field" state. This is considered acceptable given how rare it is versus the data-loss risk; D's per-key merge means individual fields can still be cleared.

@italovalcy italovalcy marked this pull request as ready for review July 11, 2026 09:52
@italovalcy italovalcy merged commit f28b176 into main Jul 11, 2026
3 checks passed
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.

1 participant