Bug 11563 - Class noEnterSubmit no longer functioning
Summary: Class noEnterSubmit no longer functioning
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: 3.16
Hardware: All All
: P2 major (vote)
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Keywords:
: 11521 (view as bug list)
Depends on: 11521
Blocks:
  Show dependency treegraph
 
Reported: 2014-01-15 15:33 UTC by Kyle M Hall
Modified: 2015-08-10 14:38 UTC (History)
8 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 11563 - Class noEnterSubmit no longer functioning (2.41 KB, patch)
2014-02-25 11:28 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 11563 - Class noEnterSubmit no longer functioning (2.47 KB, patch)
2014-02-26 17:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11563 [QA Followup] - Change .live() to .on() (1.10 KB, patch)
2014-02-26 17:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11563 [QA Followup] - Disable add noEnterSubmit to pulldowns (2.06 KB, patch)
2014-03-07 14:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11563 - Class noEnterSubmit no longer functioning (2.54 KB, patch)
2014-03-19 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11563 [QA Followup] - Change .live() to .on() (1.16 KB, patch)
2014-03-19 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11563 [QA Followup] - Disable add noEnterSubmit to pulldowns (2.15 KB, patch)
2014-03-19 09:11 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11563: (followup) Disable add noEnterSubmit to pulldowns (3.34 KB, patch)
2014-03-25 09:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 11563 - Class noEnterSubmit no longer functioning (2.60 KB, patch)
2014-03-25 15:32 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11563 [QA Followup] - Change .live() to .on() (1.22 KB, patch)
2014-03-25 15:33 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11563 [QA Followup] - Disable add noEnterSubmit to pulldowns (2.18 KB, patch)
2014-03-25 15:33 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11563: (followup) Disable add noEnterSubmit to pulldowns (3.35 KB, patch)
2014-03-25 15:33 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 11563 - Class noEnterSubmit no longer functioning (2.65 KB, patch)
2014-03-25 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11563 [QA Followup] - Change .live() to .on() (1.28 KB, patch)
2014-03-25 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11563 [QA Followup] - Disable add noEnterSubmit to pulldowns (2.27 KB, patch)
2014-03-25 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11563: (followup) Disable add noEnterSubmit to pulldowns (3.46 KB, patch)
2014-03-25 20:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11563 - Class noEnterSubmit no longer functioning (2.74 KB, patch)
2014-03-25 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11563 [QA Followup] - Change .live() to .on() (1.28 KB, patch)
2014-03-25 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11563 [QA Followup] - Disable add noEnterSubmit to pulldowns (2.27 KB, patch)
2014-03-25 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11563: (followup) Disable add noEnterSubmit to pulldowns (3.46 KB, patch)
2014-03-25 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2014-01-15 15:33:56 UTC
For some reason, even if an input as the class noEnterSubmit, the code found in staff-global.js for preventing submission when hitting the enter key is no longer triggered!
Comment 1 Christopher Brannon 2014-01-15 16:34:27 UTC
This behavior is not present in 3.12.7, but it is present on 3.15 in the sandboxes.  I do not know if this is affecting 3.14.

Christopher
Comment 2 Liz Rea 2014-01-17 09:50:06 UTC
This probably also has to do with the report of scanners submitting new borrowers early in 3.14.
Comment 3 Christopher Brannon 2014-01-17 15:25:19 UTC
(In reply to Liz Rea from comment #2)
> This probably also has to do with the report of scanners submitting new
> borrowers early in 3.14.

I would agree with that assessment, since scanner input often sends a CR signal.

Christopher
Comment 4 Fridolin Somers 2014-02-25 11:28:05 UTC Comment hidden (obsolete)
Comment 5 Fridolin Somers 2014-02-25 11:29:44 UTC
Bug 11521 is a duplicate no ?
Comment 6 Kyle M Hall 2014-02-26 17:43:32 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2014-02-26 17:44:13 UTC
*** Bug 11521 has been marked as a duplicate of this bug. ***
Comment 8 Kyle M Hall 2014-02-26 17:52:47 UTC Comment hidden (obsolete)
Comment 9 Fridolin Somers 2014-02-26 18:08:22 UTC
Indeed, live is deprecated in jQuery 1.7 https://api.jquery.com/live/
Comment 10 Kyle M Hall 2014-03-07 14:08:07 UTC Comment hidden (obsolete)
Comment 11 Fridolin Somers 2014-03-19 08:07:24 UTC
(In reply to Kyle M Hall from comment #10)
> Created attachment 25935 [details] [review] [review]
> Bug 11563 [QA Followup] - Disable add noEnterSubmit to pulldowns
> 
> When using Chrome on Mac OS X, form submits are triggered by hitting
> enter when focused on a select. This is not an issue with any other
> combincation of browser and operating system that I am aware of.
In this case, shall we add noEnterSubmit on select also ?
I think it does not impact other browser/OS combination.
Comment 12 Jonathan Druart 2014-03-19 09:10:54 UTC
QA comment:

I did not test on all pages and I only test with Firefox, it works!

Marked as Passed QA.
Comment 13 Jonathan Druart 2014-03-19 09:11:15 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2014-03-19 09:11:21 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2014-03-19 09:11:27 UTC Comment hidden (obsolete)
Comment 16 Jonathan Druart 2014-03-19 09:14:43 UTC
(In reply to Fridolin SOMERS from comment #11)
> In this case, shall we add noEnterSubmit on select also ?
> I think it does not impact other browser/OS combination.

It is not what the last patch does?
Comment 17 Fridolin Somers 2014-03-24 14:51:19 UTC
(In reply to Jonathan Druart from comment #16)
> (In reply to Fridolin SOMERS from comment #11)
> > In this case, shall we add noEnterSubmit on select also ?
> > I think it does not impact other browser/OS combination.
> 
> It is not what the last patch does?
No, this patch corrects behaviors for elements with "noEnterSubmit" class.
But this class in actually only set on "input" elements : 
  $("fieldset.rows input").addClass("noEnterSubmit");
Comment 18 Jonathan Druart 2014-03-24 15:42:42 UTC
(In reply to Fridolin SOMERS from comment #17)
> But this class in actually only set on "input" elements : 
>   $("fieldset.rows input").addClass("noEnterSubmit");

No, the *last* patch does the following :
-            $("fieldset.rows input").addClass("noEnterSubmit");
+            $("fieldset.rows input,select").addClass("noEnterSubmit");

:)
Comment 19 Fridolin Somers 2014-03-24 15:59:09 UTC
(In reply to Jonathan Druart from comment #18)
> (In reply to Fridolin SOMERS from comment #17)
> > But this class in actually only set on "input" elements : 
> >   $("fieldset.rows input").addClass("noEnterSubmit");
> 
> No, the *last* patch does the following :
> -            $("fieldset.rows input").addClass("noEnterSubmit");
> +            $("fieldset.rows input,select").addClass("noEnterSubmit");
> 
> :)
Ohhhhhh, sorry I had not seen this patch.

@Kyle :
I think the JQuery selector is not correct :
  $("fieldset.rows input,select").addClass("noEnterSubmit");
should be :
  $("fieldset.rows input, fieldset.rows select").addClass("noEnterSubmit");

Also, the pages mancredit.tt and maninvoice.tt are also using noEnterSubmit class. They should have the same JS code no ?
Comment 20 Jonathan Druart 2014-03-25 09:06:36 UTC Comment hidden (obsolete)
Comment 21 Christopher Brannon 2014-03-25 14:54:40 UTC
I can't sign off on this.  I tested memberentry.pl, and the class is not added:

<input type="submit" value="Save" onclick="return check_form_borrowers();" name="save">

mancredit.pl also fails:

<input type="submit" value="Add credit" name="add">

It does not look like the jquery is applying the modification.

Christopher
Comment 22 Christopher Brannon 2014-03-25 15:11:19 UTC
I think the correct code would read:

$("fieldset input, fieldset select").addClass("noEnterSubmit");

The .row misdirects the selector.  I've tested this, and this applies correctly.  However, I am not a patch writer.  I'll leave that to you guys.

Christopher
Comment 23 Jonathan Druart 2014-03-25 15:17:56 UTC
The class noEnterSubmit should be added to input and select elements in the form. Not on the submit button/input.
Comment 24 Christopher Brannon 2014-03-25 15:29:01 UTC
Well, ignore my comments.  For some reason, it wasn't working at first.  Now it is.  Sorry guys.  Don't know why this happens.  I can confirm it works now.
Comment 25 Biblibre Sandboxes 2014-03-25 15:32:12 UTC
Patch tested with a sandbox, by Christopher Brannon <cbrannon@cdalibrary.org>
Comment 26 Biblibre Sandboxes 2014-03-25 15:32:58 UTC Comment hidden (obsolete)
Comment 27 Biblibre Sandboxes 2014-03-25 15:33:03 UTC Comment hidden (obsolete)
Comment 28 Biblibre Sandboxes 2014-03-25 15:33:08 UTC Comment hidden (obsolete)
Comment 29 Biblibre Sandboxes 2014-03-25 15:33:12 UTC Comment hidden (obsolete)
Comment 30 Katrin Fischer 2014-03-25 20:51:13 UTC Comment hidden (obsolete)
Comment 31 Katrin Fischer 2014-03-25 20:51:18 UTC Comment hidden (obsolete)
Comment 32 Katrin Fischer 2014-03-25 20:51:21 UTC Comment hidden (obsolete)
Comment 33 Katrin Fischer 2014-03-25 20:51:26 UTC Comment hidden (obsolete)
Comment 34 Katrin Fischer 2014-03-25 20:58:21 UTC
Created attachment 26570 [details] [review]
[PASSED QA] Bug 11563 - Class noEnterSubmit no longer functioning

For some reason, even if an input as the class noEnterSubmit, the code found in staff-global.js for preventing submission when hitting the enter key is no longer triggered.
This is because this class is added using JavaScript in $(document).ready() and the keypress event listerning is also set using $(document).ready().

This patch corrects by using JQuery live() with will set event listerning event for HTML generated with JavaScript.
Alos sets noEnterSubmit to use the checkEnter(e) function, containing a more correct code.
Also corrects a small bug, for IE, window.event must be used, not event alone (even if IE is not supported for intranet).

Test plan :
- Flush browser cache (Crtl+F5) to update js files
- Go to patron creation : members/memberentrygen.tt
- Type a text in all mandatory inputs
- Type some caracters in a non mandatory input
- Press Enter key
=> Without patch : the form is submitted
=> With patch : the form is not submitted
- Try to set several lines in a textarea
=> It works (checks that in this case enter key is allowed)
- Test other pages usign noEnterSubmit class :
cataloguing/additem.tt
course_reserves/course.tt
members/mancredit.tt
members/maninvoice.tt
patron_lists/list.tt

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, tested in Firefox and Chromium.
Passes tests and QA script.
Comment 35 Katrin Fischer 2014-03-25 20:58:26 UTC
Created attachment 26571 [details] [review]
[PASSED QA] Bug 11563 [QA Followup] - Change .live() to .on()

The jquery event handler .live() has been deprecated. It is recommended
that .on() be used instead.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 36 Katrin Fischer 2014-03-25 20:58:30 UTC
Created attachment 26572 [details] [review]
[PASSED QA] Bug 11563 [QA Followup] - Disable add noEnterSubmit to pulldowns

When using Chrome on Mac OS X, form submits are triggered by hitting
enter when focused on a select. This is not an issue with any other
combincation of browser and operating system that I am aware of.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 37 Katrin Fischer 2014-03-25 20:58:35 UTC
Created attachment 26573 [details] [review]
[PASSED QA] Bug 11563: (followup) Disable add noEnterSubmit to pulldowns

Test plan:
To test on modified pages:
- Press enter when cursor is on input or select and verify the form is not
submitted.

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 38 Galen Charlton 2014-04-11 18:04:56 UTC
Pushed to master.  Thanks, Fridolin!
Comment 39 Fridolin Somers 2014-05-07 16:43:15 UTC
Pushed to 3.14.x, will be in 3.14.07
Comment 40 Claudio Costales 2014-10-13 02:11:20 UTC
Same problem in 3.16.4. 

Needs to fix file intranet-tmpl/prog/*LANG*/js/cataloging.js 

In LINE 486 erase 'noEnterSubmit' parameter in function addClass.


-    $(".input_marceditor, .indicator").addClass('noEnterSubmit');
+    $(".input_marceditor, .indicator").addClass('');
Comment 41 Katrin Fischer 2014-10-13 22:57:27 UTC
Hi Bondiurbano, can you please open a separate bug (linking to this) with a problem description?

I think if the problem is in 3.16.4 it's likely also in master, so this version should be used.
Comment 42 Claudio Costales 2014-10-13 23:40:37 UTC
(In reply to Katrin Fischer from comment #41)
> Hi Bondiurbano, can you please open a separate bug (linking to this) with a
> problem description?
> 
> I think if the problem is in 3.16.4 it's likely also in master, so this
> version should be used.

DONE! See bug 13080 addClass('noEnterSubmit') no longer functioning for 5xx tags while cataloging 

Thank you Katrin!