Description
Josef Moravec
2017-10-13 05:59:17 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"> 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> *** Bug 19491 has been marked as a duplicate of this bug. *** 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> 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>'` 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"> (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. 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"> Worked as planned, QA tool green across the board. *** Bug 19553 has been marked as a duplicate of this bug. *** (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. 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> Pushed to master for 18.05, thanks to everybody involved! Pushed to stable for 17.11.01, awesome work all! I will push to next 17.05.x release because of string freeze. Ah the patch does not apply easily. I it not a major problem, I choose not to backport to 17.05.x. |