[Bug 40643] New: circulation.tt attaches event listeners to keypress in a problematic way
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Bug ID: 40643 Summary: circulation.tt attaches event listeners to keypress in a problematic way Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: paul.derscheid@lmscloud.de QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle@bywatersolutions.com 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... -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 --- Comment #1 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Created attachment 185357 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185357&action=edit 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |phil@chetcolibrary.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #2 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Will this prevent double submission? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 --- Comment #3 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Yes. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 --- Comment #4 from Paul Derscheid <paul.derscheid@lmscloud.de> --- But best if more people have a look. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |me@paulderscheid.xyz |ity.org | CC| |lisette@bywatersolutions.co | |m -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Philip Orr <philip.orr@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Philip Orr <philip.orr@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185357|0 |1 is obsolete| | --- Comment #5 from Philip Orr <philip.orr@lmscloud.de> --- Created attachment 185494 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185494&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #185494|0 |1 is obsolete| | --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 185888 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185888&action=edit 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> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl QA Contact|testopia@bugs.koha-communit |m.de.rooy@rijksmuseum.nl |y.org | --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Could this be used in more places? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 --- Comment #8 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Yes, but it seems it's only part of the problem. I think circulation.{pl,tt} has more of these problems. I get it, though. It's one of the center pieces and therefore has many things bolted onto it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 --- Comment #9 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|25.11.00 |25.11.00,25.05.04 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 --- Comment #10 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|25.11.00,25.05.04 |25.11.00,25.05.04,24.11.10 released in| | CC| |fridolin.somers@biblibre.co | |m --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 24.11.x for 24.11.10 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40643 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |Needs documenting --- Comment #12 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11.x as it wasn't backported to 24.05.x -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org