Bug 40643 - circulation.tt attaches event listeners to keypress in a problematic way
Summary: circulation.tt attaches event listeners to keypress in a problematic way
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Paul Derscheid
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-08-13 08:36 UTC by Paul Derscheid
Modified: 2025-08-18 09:07 UTC (History)
5 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 40643: Prevent keypress buildup in circulation (4.12 KB, patch)
2025-08-13 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 40643: Prevent keypress buildup in circulation (4.17 KB, patch)
2025-08-18 09:07 UTC, Philip Orr
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Derscheid 2025-08-13 08:36:59 UTC
We haven't yet confirmed this in production because it's not easy to validate without looking someone over the shoulder who is doing actual work with Koha, but we had some reported performance issues around circ/circulation.tt.

I then found a problematic block that attaches event listeners without any guarding on every submit event on the main form if barcode has a value, even when no navigate event is fired. This has potential to cause memory leaks and major GC runs (which is what we observed).

The overarching performance problem is almost impossible to reproduce during manual testing. You really need to use not only circulation.tt but have a natural usage pattern found at circulation desks. So, navigating back and forth around circ/circulation.tt, checking out numerous items and so on...
Comment 1 Paul Derscheid 2025-08-13 08:39:49 UTC
Created attachment 185357 [details] [review]
Bug 40643: Prevent keypress buildup in circulation

We have reports of progressive slowdown on the circulation page during
long sessions. A likely cause might be a block in `circ/circulation.tt`
that attaches a new keypress handler to `#barcode` on every submit. Some
submits do not navigate (e.g. waiting holds modal, quick slip on empty
barcode), and pages restored from the bfcache keep previously attached
handlers. Over time, listeners accumulate and the UI becomes sluggish.

This patch installs a single, namespaced keypress handler at page load,
guarded by an in-flight flag toggled on submit. The handler shows the
“Barcode submitted” modal and prevents typing only while a submit is
in flight. The flag is reset on bfcache restores via `pageshow`. This
preserves existing UX while keeping the listener count constant.

Test plan:
1. Before applying the patch, open circ/circulation.pl for any patron.
2. In the browser console, run:
   (function () {
     const el = $('#barcode')[0];
     const dataFn = jQuery._data || jQuery.data;
     const ev = (dataFn && dataFn(el, 'events')) || {};
     return (ev.keypress || []).length;
   })()
   Note the keypress handler count.
3. Enter a barcode and submit. Use the browser Back button to return.
4. Re-run the IIFE from step 2. The count has increased.
5. Repeat steps 3–4 a few times and observe the count continues to grow.
6. Apply the patch.
7. Reload the page and re-run step 2; count should be ~1.
8. Enter a barcode and submit. Use Back to return. Re-run step 2.
   The count remains ~1 across submits and back/forward navigations.
9. Confirm UX is preserved:
   - While submit is in flight, pressing keys in `#barcode` shows the
     “Barcode submitted” modal and prevents typing.
   - Waiting holds flow still shows its modal and behaves as before.
   - Submitting with an empty barcode still triggers the quick slip
     behavior per system preference and does not add handlers.
10. Optional: Simulate scanner behavior by pressing Enter rapidly after
    submit. Verify the handler count remains stable.
Comment 2 Lucas Gass (lukeg) 2025-08-13 19:44:46 UTC
Will this prevent double submission?
Comment 3 Paul Derscheid 2025-08-13 20:10:41 UTC
Yes.
Comment 4 Paul Derscheid 2025-08-13 20:11:15 UTC
But best if more people have a look.
Comment 5 Philip Orr 2025-08-18 09:07:01 UTC
Created attachment 185494 [details] [review]
Bug 40643: Prevent keypress buildup in circulation

We have reports of progressive slowdown on the circulation page during
long sessions. A likely cause might be a block in `circ/circulation.tt`
that attaches a new keypress handler to `#barcode` on every submit. Some
submits do not navigate (e.g. waiting holds modal, quick slip on empty
barcode), and pages restored from the bfcache keep previously attached
handlers. Over time, listeners accumulate and the UI becomes sluggish.

This patch installs a single, namespaced keypress handler at page load,
guarded by an in-flight flag toggled on submit. The handler shows the
“Barcode submitted” modal and prevents typing only while a submit is
in flight. The flag is reset on bfcache restores via `pageshow`. This
preserves existing UX while keeping the listener count constant.

Test plan:
1. Before applying the patch, open circ/circulation.pl for any patron.
2. In the browser console, run:
   (function () {
     const el = $('#barcode')[0];
     const dataFn = jQuery._data || jQuery.data;
     const ev = (dataFn && dataFn(el, 'events')) || {};
     return (ev.keypress || []).length;
   })()
   Note the keypress handler count.
3. Enter a barcode and submit. Use the browser Back button to return.
4. Re-run the IIFE from step 2. The count has increased.
5. Repeat steps 3–4 a few times and observe the count continues to grow.
6. Apply the patch.
7. Reload the page and re-run step 2; count should be ~1.
8. Enter a barcode and submit. Use Back to return. Re-run step 2.
   The count remains ~1 across submits and back/forward navigations.
9. Confirm UX is preserved:
   - While submit is in flight, pressing keys in `#barcode` shows the
     “Barcode submitted” modal and prevents typing.
   - Waiting holds flow still shows its modal and behaves as before.
   - Submitting with an empty barcode still triggers the quick slip
     behavior per system preference and does not add handlers.
10. Optional: Simulate scanner behavior by pressing Enter rapidly after
    submit. Verify the handler count remains stable.

Signed-off-by: krimsonkharne <philip.orr@posteo.de>