Bug 11292 - barcode scanner clicking 'save'
Summary: barcode scanner clicking 'save'
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-23 02:05 UTC by Nicole C. Engard
Modified: 2015-06-04 23:30 UTC (History)
12 users (show)

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


Attachments
Bug 11292 - barcode scanner clicking 'save' (1.30 KB, patch)
2014-05-22 12:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11292 - barcode scanner clicking 'save' (1.49 KB, patch)
2014-05-28 13:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 11292 [QA Followup] - Use noEnterSubmit instead of new keypress handler (968 bytes, patch)
2014-06-16 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2013-11-23 02:05:42 UTC
When cataloging a bib record if you scan an ISBN barcode in the barcode scanner clicks the 'save' button before the cataloging is done. This did not happen in 3.10, but is happening again in 3.12.
Comment 1 Johnny Rotten 2013-12-19 17:58:27 UTC
I'm thinking that any time you send a carriage return Koha is triggering the default page action because it's not just scanning a barcode that causes the save.  In Koha 3.12 a record is saved anytime you hit the "enter" key when the cursor is in any field in a cataloging record.

George
Comment 2 Katrin Fischer 2014-03-25 21:09:36 UTC
I just tested 11563 for QA, but it doesn't fix this specific problem.
Comment 3 jdemuth@roseville.ca.us 2014-03-31 21:23:45 UTC
I wrote a script and put in the the intranetusersjs field that removes the noEnterSubmit function from specific fields. This is now allowing us to scan barcodes without it try to save automatically because of the carriage return set in the scanners. 

//Remove noEnterSubmit from fields - JD
$(document).ready(function(){
$("#userid").removeClass("noEnterSubmit");
$("#cardnumber").removeClass("noEnterSubmit");
$("#subfield952p637072").removeClass("noEnterSubmit");
});
Comment 4 Marjorie Barry-Vila 2014-04-01 13:49:34 UTC
(In reply to jdemuth@roseville.ca.us from comment #3)
> I wrote a script and put in the the intranetusersjs field that removes the
> noEnterSubmit function from specific fields. This is now allowing us to scan
> barcodes without it try to save automatically because of the carriage return
> set in the scanners. 
> 
> //Remove noEnterSubmit from fields - JD
> $(document).ready(function(){
> $("#userid").removeClass("noEnterSubmit");
> $("#cardnumber").removeClass("noEnterSubmit");
> $("#subfield952p637072").removeClass("noEnterSubmit");
> });

Thank you for the script but it is not working for me.
In wich version of koha you tested it?

Regards,
Marjorie
Comment 5 jdemuth@roseville.ca.us 2014-04-01 20:23:17 UTC
Sorry, I forgot to include we just upgraded to 3.14.4. I did notice that it is not "saving" the page which it great, but it is losing focus. I think I know how to fix that and I will test tonight and If it works I will include the updated script. 

Jen
Comment 6 Kyle M Hall 2014-05-22 12:38:14 UTC Comment hidden (obsolete)
Comment 7 Biblibre Sandboxes 2014-05-22 17:30:50 UTC
Patch tested with a sandbox, by Christopher Brannon <cbrannon@cdalibrary.org>
Comment 8 Christopher Brannon 2014-05-22 17:32:06 UTC
Patch applied cleanly and worked as outlined.

Christopher
Comment 9 Marcel de Rooy 2014-05-28 13:39:39 UTC
Created attachment 28518 [details] [review]
Bug 11292 - barcode scanner clicking 'save'

When cataloging a bib record if you scan an ISBN barcode in the barcode
scanner clicks the 'save' button before the cataloging is done. This did
not happen in 3.10, but is happening again in 3.12.

Test Plan:
1) Open the marc editor
2) Focus on a field, hit the enter key
3) Note the record tries to save
4) Focus on an indicator, hit the enter key
5) Note the record tries to save
6) Apply this patch
7) Repeat steps 2 and 4, note the record no longer
   tries to save upon carriage return

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Patch tested with a sandbox.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Jonathan Druart 2014-05-28 13:46:16 UTC
Shouldn't we add the noEnterSubmit class to the elements?
Comment 11 Marcel de Rooy 2014-05-28 14:21:47 UTC
(In reply to Jonathan Druart from comment #10)
> Shouldn't we add the noEnterSubmit class to the elements?

This can be pushed in its current state.
Note that it is referring to cataloging.js instead of staff-global.js where a keypress is active on class noEnterSubmit.
I agree that another report could bring these two closer (achieving more consistency..)
Comment 12 Galen Charlton 2014-05-28 16:26:03 UTC
(In reply to M. de Rooy from comment #11)
> (In reply to Jonathan Druart from comment #10)
> > Shouldn't we add the noEnterSubmit class to the elements?

Or to the form itself. That would be a more minimal fix, and more consistent.

> This can be pushed in its current state.
> Note that it is referring to cataloging.js instead of staff-global.js where
> a keypress is active on class noEnterSubmit.

Marcel, have you tested /all/ of the interfaces that this patch touches?  By changing cataloging.js and matching on .input_marceditor, a bunch of forms are affected, not just the main cataloging editor:

koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt
koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt
koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/additem.tt (and here accepting this patch means that there would be /two/ handlers trying to prevent form submission via the enter key)
koha-tmpl/intranet-tmpl/prog/en/modules/serials/serials-edit.tt
koha-tmpl/intranet-tmpl/prog/en/modules/tools/batchMod-edit.tt

Now, that isn't necessarily a problem -- as arguably none of these forms should be submittable using the enter key, but this patch is an example of one where the test plan does not account for all of the side effects.

> I agree that another report could bring these two closer (achieving more
> consistency..)

Why wait?  Why not insist on consistency now as opposed to adding inconsistency?

This doesn't necessarily /require/ a counter-patch now, but I am setting back to 'signed off' pending confirmation that all of the forms that this patch affects have been tested, particularly the item editor.
Comment 13 Marcel de Rooy 2014-06-02 07:54:14 UTC
(In reply to Galen Charlton from comment #12)
> Marcel, have you tested /all/ of the interfaces that this patch touches?  By
> changing cataloging.js and matching on .input_marceditor, a bunch of forms
> are affected, not just the main cataloging editor:

I noticed that it impacts more than the editor (biblio/items); I additionally looked at the acquisition forms for that reason. (Hopefully, the one who signed off, did some testing too :)
But for consistency, I certainly do not mind further improvements here. Especially if the author could amend or send a follow-up. But if not, there is no reason imo to block this patch for that reason.

BTW I additionally checked now: authorities.pl, serials-edit.pl and batchMod.pl. No problems encountered.

Back to Passed QA.

Kyle: Could you please respond about your possibilities to amend/adjust in light of the above?
Comment 14 Kyle M Hall 2014-06-02 11:36:33 UTC
I am more than willing to make adjustments if necessary. However, I'm not grokking what the inconsistency in this case is. Should be make noEnterSubmit apply to forms Koha-wide, and have a provision to allow it ( i.e. allowEnterSubmit )?
Comment 15 Marcel de Rooy 2014-06-02 13:07:27 UTC
(In reply to Kyle M Hall from comment #14)
> I am more than willing to make adjustments if necessary. However, I'm not
> grokking what the inconsistency in this case is. Should be make
> noEnterSubmit apply to forms Koha-wide, and have a provision to allow it (
> i.e. allowEnterSubmit )?

To my understanding: the inconsistency is two ways to handle the same. You define a separate keyboard handler, but elsewhere the class name noEnterSubmit is picked up by a global keyboard handler (see staff-global.js).
It would be more consistent to do the same in cataloging.js.
Comment 16 Kyle M Hall 2014-06-16 11:31:07 UTC
Created attachment 28863 [details] [review]
Bug 11292 [QA Followup] - Use noEnterSubmit instead of new keypress handler

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 17 Tomás Cohen Arazi 2014-06-24 12:45:13 UTC
Patches pushed to master.

Thanks Kyle!
Comment 18 Galen Charlton 2014-06-25 22:35:29 UTC
Pushed to 3.16.x for inclusion in 3.16.1.
Comment 19 Fridolin Somers 2014-09-05 14:02:29 UTC
Pushed to 3.14.x, will be in 3.14.11