Summary: | Framework item plugins fire twice on Acquisition item blocks | ||
---|---|---|---|
Product: | Koha | Reporter: | Jan Kissig <bibliothek> |
Component: | Acquisitions | Assignee: | Jan Kissig <bibliothek> |
Status: | CLOSED FIXED | QA Contact: | Katrin Fischer <katrin.fischer> |
Severity: | normal | ||
Priority: | P5 - low | CC: | fridolin.somers, julian.maurice, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
23.11.00,23.05.06,22.11.12
|
Circulation function: | |
Attachments: |
Bug_35012
Bug_35012 Bug 35012: Prevent Click handler firing twice Bug 35012: DO_NOT_PUSH - Add action to barcode's Blur event Bug 35012: Apply change to all events Bug 35012: Apply change to all events Bug 35012: DO_NOT_PUSH - Add action to barcode's Blur event Bug 35012: Prevent Click handler firing twice Bug 35012: Apply change to all events |
Description
Jan Kissig
2023-10-09 12:43:26 UTC
It looks like additem.js calls the BindPluginEvents function which is registering the event listeners. Then the ready function for each plugin is also called and registers the event listeners again. Created attachment 156792 [details] [review] Bug_35012 test plan: a) build a new basket + order line using "item create on placing an order" b) in the item creation dialog (neworderempty.pl) please use the webbrowsers dev tools to analyze the [...] button next to subfield d (Date acquired) and inspect the events that get fired by clicking c) notice the Clicktag_952_subfield_d_* event gets fired twice. d) apply patch and reload page e) there should be only 1 click handler left (besides the jquery one), which points to the Clicktag_952_subfield_* function Created attachment 156793 [details] [review] Bug_35012 test plan: a) build a new basket + order line using "item create on placing an order" b) in the item creation dialog (neworderempty.pl) please use the webbrowsers dev tools to analyze the [...] button next to subfield d (Date acquired) and inspect the events that get fired by clicking c) notice the Clicktag_952_subfield_d_* event gets fired twice. d) apply patch and reload page e) there should be only 1 click handler left (besides the jquery one), which points to the Clicktag_952_subfield_* function Interesting patch ! Not sure if we are on the right track yet but thats just intuition.. Will be looking. Created attachment 158718 [details] [review] Bug 35012: Prevent Click handler firing twice test plan: a) build a new basket + order line using "item create on placing an order" b) in the item creation dialog (neworderempty.pl) please use the webbrowsers dev tools to analyze the [...] button next to subfield d (Date acquired) and inspect the events that get fired by clicking c) notice the Clicktag_952_subfield_d_* event gets fired twice. d) apply patch and reload page e) there should be only 1 click handler left (besides the jquery one), which points to the Clicktag_952_subfield_* function Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> See also bug 35298. You may have seen that the Focus event of dateaccessioned did not fire. But that is related to use of Flatpickr in this plugin. Created attachment 158719 [details] [review] Bug 35012: DO_NOT_PUSH - Add action to barcode's Blur event No test plan. See next patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 158720 [details] [review] Bug 35012: Apply change to all events We should not just look at Click. Test plan: [1] Apply the example patch for a Blur action in barcode.pl. [2] Check that barcode.pl is active in your framework and that autoBarcode is not disabled. [3] Open the browser dev console. [4] Tab a few times through barcode in the neworderempty form of Acquisition. If you leave the field, the Blur event fires. => Without this patch you will see two console lines (FF may show a little '2' icon at the right side) added. With this patch just one. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> (In reply to Julian Maurice from comment #5) > Maybe bug 30975 fix this ? Yes, definitely. I recommend to use this as a temporary fix for master and backport it to stable branches. Hopefully, we can push 30975 soon after 23.11 release. And further fix this code. Created attachment 158782 [details] [review] Bug 35012: Apply change to all events We should not just look at Click. Test plan: [1] Apply the example patch for a Blur action in barcode.pl. [2] Check that barcode.pl is active in your framework and that autoBarcode is not disabled. [3] Open the browser dev console. [4] Tab a few times through barcode in the neworderempty form of Acquisition. If you leave the field, the Blur event fires. => Without this patch you will see two console lines (FF may show a little '2' icon at the right side) added. With this patch just one. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jan Kissig <jkissig@th-wildau.de> Next time, please start the test plan with getting into the acquisition module, I was quite getting quite frustrated with the normal item form :D Created attachment 158816 [details] [review] Bug 35012: DO_NOT_PUSH - Add action to barcode's Blur event No test plan. See next patch. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 158817 [details] [review] Bug 35012: Prevent Click handler firing twice test plan: a) build a new basket + order line using "item create on placing an order" b) in the item creation dialog (neworderempty.pl) please use the webbrowsers dev tools to analyze the [...] button next to subfield d (Date acquired) and inspect the events that get fired by clicking c) notice the Clicktag_952_subfield_d_* event gets fired twice. d) apply patch and reload page e) there should be only 1 click handler left (besides the jquery one), which points to the Clicktag_952_subfield_* function Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> See also bug 35298. You may have seen that the Focus event of dateaccessioned did not fire. But that is related to use of Flatpickr in this plugin. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 158818 [details] [review] Bug 35012: Apply change to all events We should not just look at Click. Test plan: [1] Apply the example patch for a Blur action in barcode.pl. [2] Check that barcode.pl is active in your framework and that autoBarcode is not disabled. [3] Open the browser dev console. [4] Tab a few times through barcode in the neworderempty form of Acquisition. If you leave the field, the Blur event fires. => Without this patch you will see two console lines (FF may show a little '2' icon at the right side) added. With this patch just one. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jan Kissig <jkissig@th-wildau.de> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> (In reply to Katrin Fischer from comment #11) > Next time, please start the test plan with getting into the acquisition > module, I was quite getting quite frustrated with the normal item form :D Ok, I realize there is 2 test plans, maybe I should start the weekend :) (In reply to Katrin Fischer from comment #11) > Next time, please start the test plan with getting into the acquisition > module, I was quite getting quite frustrated with the normal item form :D Ah yes, that would have been helpful indeed :) Thanks. Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.06 Nice work everyone! Pushed to oldstable for 22.11.x |