Bug 21576 - Add a developer script to automatically fix missing filters
Summary: Add a developer script to automatically fix missing filters
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 13618
Blocks: 21526 22466
  Show dependency treegraph
 
Reported: 2018-10-15 20:14 UTC by Jonathan Druart
Modified: 2020-06-04 20:32 UTC (History)
5 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 21576: Add a developer script to fix missing TT filters (23.00 KB, patch)
2018-10-15 20:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21576: Keep compatibility with QA script (4.08 KB, patch)
2018-10-15 20:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21576: Keep compatibility with QA script (4.51 KB, patch)
2018-10-15 20:50 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21576: Handle complex uri (10.00 KB, patch)
2018-10-15 22:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21576: Keep compatibility with QA script (4.64 KB, patch)
2018-10-16 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21576: Handle complex uri (10.00 KB, patch)
2018-10-16 15:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21576: Add a developer script to fix missing TT filters (23.03 KB, patch)
2018-10-16 17:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21576: Keep compatibility with QA script (4.67 KB, patch)
2018-10-16 17:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21576: Handle complex uri (10.04 KB, patch)
2018-10-16 17:59 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21576: Add a developer script to fix missing TT filters (23.10 KB, patch)
2018-10-17 11:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21576: Keep compatibility with QA script (4.75 KB, patch)
2018-10-17 11:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21576: Handle complex uri (10.11 KB, patch)
2018-10-17 11:27 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21576: Preserve chomp chars for USE raw (1.50 KB, patch)
2018-10-17 14:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21576: Add a developer script to fix missing TT filters (23.10 KB, patch)
2018-10-24 11:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21576: Keep compatibility with QA script (4.75 KB, patch)
2018-10-24 11:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21576: Handle complex uri (10.11 KB, patch)
2018-10-24 11:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 21576: Preserve chomp chars for USE raw (1.57 KB, patch)
2018-10-24 11:32 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2018-10-15 20:14:05 UTC
Since bug 13618 developers have to escape TT variables when needed. It is not easy to think about (as we are lazy) and hard to pick the good one (html vs uri).
It would be great to help our brains and have a script to add them automagically when needed.
Comment 1 Jonathan Druart 2018-10-15 20:18:03 UTC
Created attachment 80640 [details] [review]
Bug 21576: Add a developer script to fix missing TT filters

See bug 13618 and bug 21526.

We need a script to add missing filters, or replace wrong ones.

Test plan:
- Add unescaped variables to .tt files
- prove xt/find-missing-filters.t
will warn about them
- perl misc/devel/add_missing_filters.pl
will add the missing/wrong filters
- prove xt/find-missing-filters.t
will now be happy
Comment 2 Jonathan Druart 2018-10-15 20:35:55 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2018-10-15 20:50:58 UTC
Created attachment 80644 [details] [review]
Bug 21576: Keep compatibility with QA script

The QA script call 'missing_filters', it sounds better to keep it in
order to avoid fixing it and have a weird condition (if
module->can('subroutine_name')) to maintain
Comment 4 Jonathan Druart 2018-10-15 22:51:10 UTC
Created attachment 80648 [details] [review]
Bug 21576: Handle complex uri

Only the first TT params in a href was taken into account.
This also takes care to replace into href attributes only (for instance not
title, etc.)
Comment 5 Jonathan Druart 2018-10-16 15:23:36 UTC
Created attachment 80659 [details] [review]
Bug 21576: Keep compatibility with QA script

The QA script call 'missing_filters', it sounds better to keep it in
order to avoid fixing it and have a weird condition (if
module->can('subroutine_name')) to maintain
Comment 6 Jonathan Druart 2018-10-16 15:23:40 UTC
Created attachment 80660 [details] [review]
Bug 21576: Handle complex uri

Only the first TT params in a href was taken into account.
This also takes care to replace into href attributes only (for instance not
title, etc.)
Comment 7 Owen Leonard 2018-10-16 17:59:46 UTC
Created attachment 80671 [details] [review]
Bug 21576: Add a developer script to fix missing TT filters

See bug 13618 and bug 21526.

We need a script to add missing filters, or replace wrong ones.

Test plan:
- Add unescaped variables to .tt files
- prove xt/find-missing-filters.t
will warn about them
- perl misc/devel/add_missing_filters.pl
will add the missing/wrong filters
- prove xt/find-missing-filters.t
will now be happy

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 8 Owen Leonard 2018-10-16 17:59:50 UTC
Created attachment 80672 [details] [review]
Bug 21576: Keep compatibility with QA script

The QA script call 'missing_filters', it sounds better to keep it in
order to avoid fixing it and have a weird condition (if
module->can('subroutine_name')) to maintain

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Owen Leonard 2018-10-16 17:59:54 UTC
Created attachment 80673 [details] [review]
Bug 21576: Handle complex uri

Only the first TT params in a href was taken into account.
This also takes care to replace into href attributes only (for instance not
title, etc.)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Martin Renvoize 2018-10-17 11:26:59 UTC
Created attachment 80694 [details] [review]
Bug 21576: Add a developer script to fix missing TT filters

See bug 13618 and bug 21526.

We need a script to add missing filters, or replace wrong ones.

Test plan:
- Add unescaped variables to .tt files
- prove xt/find-missing-filters.t
will warn about them
- perl misc/devel/add_missing_filters.pl
will add the missing/wrong filters
- prove xt/find-missing-filters.t
will now be happy

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2018-10-17 11:27:03 UTC
Created attachment 80695 [details] [review]
Bug 21576: Keep compatibility with QA script

The QA script call 'missing_filters', it sounds better to keep it in
order to avoid fixing it and have a weird condition (if
module->can('subroutine_name')) to maintain

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2018-10-17 11:27:07 UTC
Created attachment 80696 [details] [review]
Bug 21576: Handle complex uri

Only the first TT params in a href was taken into account.
This also takes care to replace into href attributes only (for instance not
title, etc.)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2018-10-17 11:27:48 UTC
Awesome work making the devs lives easier Jonathan.

Passing QA.. reads well and works as expected.
Comment 14 Jonathan Druart 2018-10-17 14:30:30 UTC
Created attachment 80716 [details] [review]
Bug 21576: Preserve chomp chars for USE raw

See bug 21526 comments 5-7

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Martin Renvoize 2018-10-24 11:32:19 UTC
Created attachment 81060 [details] [review]
Bug 21576: Add a developer script to fix missing TT filters

See bug 13618 and bug 21526.

We need a script to add missing filters, or replace wrong ones.

Test plan:
- Add unescaped variables to .tt files
- prove xt/find-missing-filters.t
will warn about them
- perl misc/devel/add_missing_filters.pl
will add the missing/wrong filters
- prove xt/find-missing-filters.t
will now be happy

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2018-10-24 11:32:23 UTC
Created attachment 81061 [details] [review]
Bug 21576: Keep compatibility with QA script

The QA script call 'missing_filters', it sounds better to keep it in
order to avoid fixing it and have a weird condition (if
module->can('subroutine_name')) to maintain

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2018-10-24 11:32:26 UTC
Created attachment 81062 [details] [review]
Bug 21576: Handle complex uri

Only the first TT params in a href was taken into account.
This also takes care to replace into href attributes only (for instance not
title, etc.)

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2018-10-24 11:32:30 UTC
Created attachment 81063 [details] [review]
Bug 21576: Preserve chomp chars for USE raw

See bug 21526 comments 5-7

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 19 Martin Renvoize 2018-10-24 11:32:58 UTC
Just given the final followup a quick once over.. still Passed QA
Comment 20 Nick Clemens 2018-10-26 17:18:08 UTC
Awesome work all!

Pushed to master for 18.11
Comment 21 Martin Renvoize 2018-11-06 15:05:45 UTC
Enhancement, will not be backported to 18.05.x series.
Comment 22 Marcel de Rooy 2018-11-09 08:55:54 UTC
What about <img src=""> here ?
Comment 23 Martin Renvoize 2018-11-10 07:04:00 UTC
I think the are a few improvements that could be made here, but personally I think they should be recorded in follow-up bugs so they don't get lost in the conversion here.
Comment 24 Fridolin Somers 2019-01-02 09:50:05 UTC
Resolved ?
Comment 25 Jonathan Druart 2019-02-14 19:08:11 UTC
(In reply to Marcel de Rooy from comment #22)
> What about <img src=""> here ?

Hi Marcel, can you explain what you had in mind? I am not sure I understand.
Comment 26 Marcel de Rooy 2019-10-25 06:58:38 UTC
(In reply to Jonathan Druart from comment #25)
> (In reply to Marcel de Rooy from comment #22)
> > What about <img src=""> here ?
> 
> Hi Marcel, can you explain what you had in mind? I am not sure I understand.

I am not sure if I still do too :) Waited too long..
But I guess it is about things like:

if ( $line =~ qr{<a href="([^"]+)} ) {

Here you handle the URI in the href attribute. Which is good. But I do not see something similar for handling the src attribute of an img tag.
Probably that was my question ;)
Comment 27 Jonathan Druart 2019-10-25 08:58:05 UTC
Yes Marcel, it's a good idea. But we must catch the other tags as well.