Bug 14266

Summary:	Replace trim() with $.trim() in opac-shareshelf.tt
Product:	Koha	Reporter:	David Cook <dcook>
Component:	Templates	Assignee:	David Cook <dcook>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	normal
Priority:	P5 - low	CC:	chris, indradg, jonathan.druart, kyle, mtj, tomascohen, wizzyrea
Version:	Main
Hardware:	All
OS:	All
See Also:	http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14265
Change sponsored?:	---	Patch complexity:	---
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:
Circulation function:
Attachments:	Replace trim() with $.trim() in opac-shareshelf.tt Replace trim() with $.trim() in opac-shareshelf.tt [COUNTER-PATCH] Bug 14266: Trim the email address in the pl script Bug 14266: Trim the email address in the pl script [PASSED QA] Bug 14266: Trim the email address in the pl script

Description David Cook 2015-05-25 04:06:29 UTC

At the moment, we use both jQuery's $.trim() and the regular Javascript trim() in our templates.

You can find trim() in:
- koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-shareshelf.tt
- koha-tmpl/intranet-tmpl/prog/en/modules/admin/categorie.tt

You can find $.trim() in:
- koha-tmpl/intranet-tmpl/prog/en/modules/admin/transport-cost-matrix.tt

The jQuery $.trim() appears perfectly cross-platform, but the Javascript trim() isn't supported in IE8 or earlier. It was only introduced in IE9.

Since we already use jQuery everywhere, I don't see why we can't use $.trim() instead of trim(). 

At the moment, people using IE8 in the staff client (and I'm sure there are many of them) can't save patron categories. Javascript errors will prevent the submission of the page.

--

I know we occasionally debate supporting older versions of IE, but this is a pretty easy win.

Comment 1 David Cook 2015-05-25 04:12:47 UTC Comment hidden (obsolete)

Created attachment 39470 [details] [review]
Replace trim() with $.trim() in opac-shareshelf.tt

This patch replaces trim() with $.trim() which is supported
in versions of IE older than IE9.

_TEST PLAN_

Before applying patch:

0) Use IE 8 or Document Mode 8 in a newer IE using F12 Developer Tools
1) Set OpacAllowSharingPrivateLists to "Allow" in Global System Preferences
2) Create a private list in the OPAC
3) Add a record to the private list
4) Click "Share" or "Share list" on one of the list screens
5) Type in an email address and click "Send"
6) Note an error preventing the submission of the page (you can
also see this in the console log)

Apply the patch:

7) Hold shift + refresh the browser to update any Javascript cache
8) Try to "Share" the list again
9) Note that the form submit after clicking "Send" and
that there are no errors

Comment 2 Indranil Das Gupta 2015-05-25 13:32:22 UTC

(In reply to David Cook from comment #1)
> Created attachment 39470 [details] [review] [review]
<snipped>
> 5) Type in an email address and click "Send"
> 6) Note an error preventing the submission of the page (you can
> also see this in the console log)

Strangely I couldn't reproduce the error on IE 8.0.6001.18702
There was no problem with form submission.

Comment 3 David Cook 2015-05-25 23:40:40 UTC

(In reply to Indranil Das Gupta from comment #2)
> Strangely I couldn't reproduce the error on IE 8.0.6001.18702
> There was no problem with form submission.

Ah, my bad with the test plan. The form will still submit. It'll just produce a Javascript error along the way.

Comment 4 Indranil Das Gupta 2015-05-25 23:45:06 UTC

Created attachment 39497 [details] [review]
Replace trim() with $.trim() in opac-shareshelf.tt

This patch replaces trim() with $.trim() which is supported
in versions of IE older than IE9.

Revised test plan
=================

Before applying patch:

0) Use IE 8 or Document Mode 8 in a newer IE using F12 Developer Tools
1) Set OpacAllowSharingPrivateLists to "Allow" in Global System Preferences
2) Create a private list in the OPAC
3) Add a record to the private list
4) Click "Share" or "Share list" on one of the list screens
5) Type in an email address and click "Send"
6) Note the error in the console log
7) The page should submit

Apply the patch:

7) Hold shift + refresh the browser to update any Javascript cache
8) Try to "Share" the list again
9) Note that the form submit after clicking "Send" and
that there are no errors in the console log

http://bugs.koha-community.org/show_bug.cgi?id=14266

Signed-off-by: Indranil Das Gupta <indradg@gmail.com>
Remarks: Works as per revised test plan

Comment 5 Jonathan Druart 2015-05-26 11:09:50 UTC Comment hidden (obsolete)

Created attachment 39509 [details] [review]
[COUNTER-PATCH] Bug 14266: Trim the email address in the pl script

The original concern of bug 14266 was to provide a compatibility for
<IE9.
But actually we don't need to trim the email address template side.
It will even better to trim it in the perl script, so that the email
will be trimed even if JS is disabled.

Test plan:
1/ Share a list and does not provide any email address
2/ Submit
=> The form is not submited, no alert/message is displayed (same as
before this patch).
3/ Share a list and provide an email address with spaces before and
after
4/ Submit
=> You should receive the email

Comment 6 Jonathan Druart 2015-05-26 11:10:12 UTC

David, could you please have a look at my counter patch?

Comment 7 David Cook 2015-05-28 00:10:30 UTC

(In reply to Jonathan Druart from comment #6)
> David, could you please have a look at my counter patch?

Looks good to me :). I'm not attached to trim on the client side.

I'm not really working on Koha for the rest of the week, but I could test this next week if no one does before then.

Indranil, you could get another sign off in ;)

Comment 8 Indranil Das Gupta 2015-05-28 19:16:11 UTC Comment hidden (obsolete)

Created attachment 39647 [details] [review]
Bug 14266: Trim the email address in the pl script

The original concern of bug 14266 was to provide a compatibility for
<IE9.
But actually we don't need to trim the email address template side.
It will even better to trim it in the perl script, so that the email
will be trimed even if JS is disabled.

Test plan:
1/ Share a list and does not provide any email address
2/ Submit
=> The form is not submited, no alert/message is displayed (same as
before this patch).
3/ Share a list and provide an email address with spaces before and
after
4/ Submit
=> You should receive the email

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Test output compliant with expected test plan outcome.

Comment 9 Kyle M Hall (khall) 2015-05-29 16:01:29 UTC

Created attachment 39701 [details] [review]
[PASSED QA] Bug 14266: Trim the email address in the pl script

The original concern of bug 14266 was to provide a compatibility for
<IE9.
But actually we don't need to trim the email address template side.
It will even better to trim it in the perl script, so that the email
will be trimed even if JS is disabled.

Test plan:
1/ Share a list and does not provide any email address
2/ Submit
=> The form is not submited, no alert/message is displayed (same as
before this patch).
3/ Share a list and provide an email address with spaces before and
after
4/ Submit
=> You should receive the email

Signed-off-by: Indranil Das Gupta (L2C2 Technologies) <indradg@gmail.com>
Test output compliant with expected test plan outcome.

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

Comment 10 Kyle M Hall (khall) 2015-05-29 16:05:56 UTC

Having trouble attaching first patch. Consider both to have passed qa. It just gets overwritten anyway.

Comment 11 Tomás Cohen Arazi (tcohen) 2015-06-01 17:20:19 UTC

Patches pushed to master.

Thankas David!

Comment 12 Chris Cormack 2015-06-03 08:03:22 UTC

Pushed to 3.20.x will be in 3.20.1

Comment 13 Liz Rea 2015-06-22 22:39:52 UTC

Patch pushed to 3.18.x will be in 3.18.08

Comment 14 Mason James 2015-07-25 09:52:32 UTC

Pushed to 3.16.x, will be in 3.16.13