Bug 19456

Summary: Some pages title tag contains html
Product: Koha Reporter: Josef Moravec <josef.moravec>
Component: Staff interfaceAssignee: Josef Moravec <josef.moravec>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: normal    
Priority: P5 - low CC: aleisha, david.bourgault, fridolin.somers, gmcharlt, jonathan.druart, kyle, nick, simon.pouchol, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14919
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 14039    
Bug Blocks: 18403    
Attachments: Bug19456: Make patron-title ability to be generated with or without html tags
Bug19456: Make patron-title ability to be generated with or without html tags
Bug 19456: Make patron-title ability to be generated with or without html tags
Bug 19456: Make patron-title ability to be generated with or without html tags
Bug 19456: Make patron-title ability to be generated with or without html tags
Bug 19456: Make patron-title ability to be generated with or without html tags

Description Josef Moravec 2017-10-13 05:59:17 UTC
This comes from patron-title.inc
Comment 1 Josef Moravec 2017-10-13 06:51:33 UTC
Created attachment 68044 [details] [review]
Bug19456: Make patron-title ability to be generated with or without html tags

Modified pages:
circ/circulation.pl
circ/circulation_batch_checkouts.pl
members/boraccount.pl
members/files.pl
members/holdshistory.pl
members/housebound.pl
members/moremember.pl
members/notices.pl
members/purchase-suggestions.pl
members/readingrec.pl
members/routing-lists.pl
members/statistics.pl
tools/viewlog.pl

Test plan:
0) Do not apply the patch
1) Have a patron with title/salution filled in
2) Confirm bug, go for example to circ/circulation page and see there is
html in <title> tag (you can see it in your browser page/window title)
3) Apply the patch
4) Go through circulation/patron pages (see modified page above) and
confirm there is no html in <title> tag, but on the page itself the
salutation should be in <span class="patron-title">
Comment 2 Simon Pouchol 2017-10-13 14:26:29 UTC
Created attachment 68081 [details] [review]
Bug19456: Make patron-title ability to be generated with or without html tags

Modified pages:
circ/circulation.pl
circ/circulation_batch_checkouts.pl
members/boraccount.pl
members/files.pl
members/holdshistory.pl
members/housebound.pl
members/moremember.pl
members/notices.pl
members/purchase-suggestions.pl
members/readingrec.pl
members/routing-lists.pl
members/statistics.pl
tools/viewlog.pl

Test plan:
0) Do not apply the patch
1) Have a patron with title/salution filled in
2) Confirm bug, go for example to circ/circulation page and see there is
html in <title> tag (you can see it in your browser page/window title)
3) Apply the patch
4) Go through circulation/patron pages (see modified page above) and
confirm there is no html in <title> tag, but on the page itself the
salutation should be in <span class="patron-title">

Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
Comment 3 Aleisha Amohia 2017-10-18 22:00:59 UTC
*** Bug 19491 has been marked as a duplicate of this bug. ***
Comment 4 Kyle M Hall 2017-10-27 13:28:43 UTC
Created attachment 68702 [details] [review]
Bug 19456: Make patron-title ability to be generated with or without html tags

Modified pages:
circ/circulation.pl
circ/circulation_batch_checkouts.pl
members/boraccount.pl
members/files.pl
members/holdshistory.pl
members/housebound.pl
members/moremember.pl
members/notices.pl
members/purchase-suggestions.pl
members/readingrec.pl
members/routing-lists.pl
members/statistics.pl
tools/viewlog.pl

Test plan:
0) Do not apply the patch
1) Have a patron with title/salution filled in
2) Confirm bug, go for example to circ/circulation page and see there is
html in <title> tag (you can see it in your browser page/window title)
3) Apply the patch
4) Go through circulation/patron pages (see modified page above) and
confirm there is no html in <title> tag, but on the page itself the
salutation should be in <span class="patron-title">

Signed-off-by: Simon Pouchol <simon.pouchol@biblibre.com>
Comment 5 Jonathan Druart 2017-10-27 14:43:53 UTC
1/ QA script detected a missing %
 FAIL   koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt
   FAIL   valid_template
                parse error - /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt line 109: unexpected token (])
  [% INCLUDE 'patron-title.inc' invert_name = 1 use_html = 1 ] [% END %]


2/ So basically we want use_html everywhere but in title tags, right?
If this is correct, I would prefer to do the reverse: add a do_not_use_html flag when patron-title.inc is used in title tags. That could be easily caught with `git grep patron-title.inc|grep '<title>'`
Comment 6 Josef Moravec 2017-10-27 19:13:37 UTC
Created attachment 68787 [details] [review]
Bug 19456: Make patron-title ability to be generated with or without html tags

Modified pages:
circ/circulation.pl
circ/circulation_batch_checkouts.pl
members/boraccount.pl
members/files.pl
members/holdshistory.pl
members/housebound.pl
members/moremember.pl
members/notices.pl
members/purchase-suggestions.pl
members/readingrec.pl
members/routing-lists.pl
members/statistics.pl

Test plan:
0) Do not apply the patch
1) Have a patron with title/salution filled in
2) Confirm bug, go for example to circ/circulation page and see there is
html in <title> tag (you can see it in your browser page/window title)
3) Apply the patch
4) Go through circulation/patron pages (see modified page above) and
confirm there is no html in <title> tag, but on the page itself the
salutation should be in <span class="patron-title">
Comment 7 Josef Moravec 2017-10-27 19:14:35 UTC
(In reply to Jonathan Druart from comment #5)
> 1/ QA script detected a missing %
>  FAIL  
> koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt
>    FAIL   valid_template
>                 parse error -
> /home/vagrant/kohaclone/koha-tmpl/intranet-tmpl/prog/en/modules/circ/
> circulation_batch_checkouts.tt line 109: unexpected token (])
>   [% INCLUDE 'patron-title.inc' invert_name = 1 use_html = 1 ] [% END %]
> 

Fixed in new patch, now it passes qa tools

> 
> 2/ So basically we want use_html everywhere but in title tags, right?
> If this is correct, I would prefer to do the reverse: add a do_not_use_html
> flag when patron-title.inc is used in title tags. That could be easily
> caught with `git grep patron-title.inc|grep '<title>'`

You are right, the ratio is 23 in page content versus 12 in title tag
I changed it.

Followup would rewrite almost everything anyway, so I decided to make new patch and ask again for sign off.
Comment 8 David Bourgault 2017-10-27 19:54:27 UTC
Created attachment 68790 [details] [review]
Bug 19456: Make patron-title ability to be generated with or without html tags

Modified pages:
circ/circulation.pl
circ/circulation_batch_checkouts.pl
members/boraccount.pl
members/files.pl
members/holdshistory.pl
members/housebound.pl
members/moremember.pl
members/notices.pl
members/purchase-suggestions.pl
members/readingrec.pl
members/routing-lists.pl
members/statistics.pl

Test plan:
0) Do not apply the patch
1) Have a patron with title/salution filled in
2) Confirm bug, go for example to circ/circulation page and see there is
html in <title> tag (you can see it in your browser page/window title)
3) Apply the patch
4) Go through circulation/patron pages (see modified page above) and
confirm there is no html in <title> tag, but on the page itself the
salutation should be in <span class="patron-title">
Comment 9 David Bourgault 2017-10-27 19:55:02 UTC
Worked as planned, QA tool green across the board.
Comment 10 Jonathan Druart 2017-11-02 11:24:48 UTC
*** Bug 19553 has been marked as a duplicate of this bug. ***
Comment 11 Jonathan Druart 2017-11-02 11:40:24 UTC
(In reply to David Bourgault from comment #9)
> Worked as planned, QA tool green across the board.

Do not forget to add your 'signed-off-by' line to the patch.
Comment 12 Nick Clemens (kidclamp) 2017-11-13 16:54:02 UTC
Created attachment 69131 [details] [review]
Bug 19456: Make patron-title ability to be generated with or without html tags

Modified pages:
circ/circulation.pl
circ/circulation_batch_checkouts.pl
members/boraccount.pl
members/files.pl
members/holdshistory.pl
members/housebound.pl
members/moremember.pl
members/notices.pl
members/purchase-suggestions.pl
members/readingrec.pl
members/routing-lists.pl
members/statistics.pl

Test plan:
0) Do not apply the patch
1) Have a patron with title/salution filled in
2) Confirm bug, go for example to circ/circulation page and see there is
html in <title> tag (you can see it in your browser page/window title)
3) Apply the patch
4) Go through circulation/patron pages (see modified page above) and
confirm there is no html in <title> tag, but on the page itself the
salutation should be in <span class="patron-title">

Signed-off-by: David Bourgalt <david.bourgault@inlibro.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Jonathan Druart 2017-12-07 12:39:53 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 14 Nick Clemens (kidclamp) 2017-12-14 12:30:23 UTC
Pushed to stable for 17.11.01, awesome work all!
Comment 15 Fridolin Somers 2017-12-20 08:12:38 UTC
I will push to next 17.05.x release because of string freeze.
Comment 16 Fridolin Somers 2018-01-03 13:41:55 UTC
Ah the patch does not apply easily.
I it not a major problem, I choose not to backport to 17.05.x.