Bug 10310

Summary: Prevent submit form pressing enter should be improved
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Staff interfaceAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: bgkriegel, chrish, gmcharlt, m.de.rooy, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 9507    
Bug Blocks:    
Attachments: Bug 10310: Prevent submitting form with enter does not work with IE
Bug 10310: Prevent submitting form with enter does not work with IE
Bug 10310: Prevent submitting form with enter does not work with IE
Bug 10310: Prevent submitting form with enter does not work with IE
Bug 10310: Followup Prevent submitting form with enter does not work with IE
Bug 10310: Prevent submitting form with enter does not work with IE
Bug 10310: Followup Prevent submitting form with enter does not work with IE

Description Jonathan Druart 2013-05-22 13:48:39 UTC
Actually it does not work for IE (see Marcel's comment on bug 9507).
Comment 1 Jonathan Druart 2013-05-22 13:51:31 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2013-05-22 14:01:07 UTC Comment hidden (obsolete)
Comment 3 Marcel de Rooy 2013-05-22 14:12:24 UTC
QA Comment:
Very quick! fix from Jonathan as followup for 9507.
Tested neworderempty on FF and IE.
Grepped prevent_submit and checked templates for doc3 tag.
Notes textarea still worked with Enter.

Good improvement and reduction of code. Seems to be browser independent.
Two document ready functions are not a problem for jQuery too.

Passed QA
Comment 4 Owen Leonard 2013-05-22 14:16:00 UTC
This seems to work in IE9, but I think it has an unintended consequence: You can't tab to the submit button and hit <enter> to submit.
Comment 5 Marcel de Rooy 2013-05-22 14:17:46 UTC
(In reply to comment #4)
> This seems to work in IE9, but I think it has an unintended consequence: You
> can't tab to the submit button and hit <enter> to submit.

So true. This just went too fast :)
Comment 6 Jonathan Druart 2013-05-22 14:40:02 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2013-05-22 14:42:29 UTC
(In reply to comment #4)
> This seems to work in IE9, but I think it has an unintended consequence: You
> can't tab to the submit button and hit <enter> to submit.

Yep good catch Owen!

The last patch returns false only if enter is pressed on input with a type != "submit". I don't find a case where a problem can occur.
Comment 8 Marcel de Rooy 2013-05-23 06:59:49 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2013-05-23 07:00:56 UTC
Tested it again. Entering the submit button works now.
Signed off.
Comment 10 Marcel de Rooy 2013-05-23 07:12:01 UTC
My former QA comment still holds.
Good improvement.

Just curious about this line:
 $(e.target).attr('type') != "submit"
Is there a difference with using e.target.type?
Does jQuery not guarantee already that object e contains a 'which' and 'target'?
Note that you assume that in the lines before this line when you directly refer to e.which and e.target.

This tiny inconsistency does not prevent me from passing QA on this patch (second try). Just wait for Owen :)
This patch makes it work too on IE9 which did not work. I could not test on earlier versions of IE. Not encouraging IE for staff at all btw..

Passed QA
Comment 11 Jonathan Druart 2013-05-23 07:40:06 UTC
(In reply to comment #10)
> Just curious about this line:
>  $(e.target).attr('type') != "submit"
> Is there a difference with using e.target.type?
> Does jQuery not guarantee already that object e contains a 'which' and
> 'target'?

Yes, you are right, it is inconsistent. I will provide a followup.
Comment 12 Jonathan Druart 2013-05-23 07:40:50 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2013-05-23 07:41:48 UTC
Switch back to Needs Signoff
Comment 14 Marcel de Rooy 2013-05-23 08:17:10 UTC
Created attachment 18329 [details] [review]
Bug 10310: Prevent submitting form with enter does not work with IE

I am not able to test this patch with IE...
I tested it with Chromium and FF and it works great.

This patch can be tested on the neworderempty.pl page
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2013-05-23 08:17:14 UTC
Created attachment 18330 [details] [review]
Bug 10310: Followup Prevent submitting form with enter does not work with IE

Check e.target.type instead of using jQuery.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Marcel de Rooy 2013-05-23 08:18:04 UTC
Nothing to say :) Back to Passed QA
Comment 17 Galen Charlton 2013-05-23 14:41:17 UTC
Pushed to master.  Thanks, Jonathan!
Comment 18 Tomás Cohen Arazi 2013-06-03 14:19:03 UTC
Nice fix, thanks Jonathan!

This patch has been pushed to 3.12.x, will be in 3.12.1.
Comment 19 Bernardo Gonzalez Kriegel 2013-06-06 01:15:59 UTC
Pushed to 3.10.x, will be in 3.10.7
Comment 20 Chris Hall 2013-06-12 08:10:06 UTC
Apply perfectly since the 9507 fixup.
Pushed both patches to 3.8.x, will be in 3.8.14.