Bug 33551 - Rogue span in patron restriction types admin page title
Summary: Rogue span in patron restriction types admin page title
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Lucas Gass (lukeg)
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 23681
Blocks:
  Show dependency treegraph
 
Reported: 2023-04-17 20:52 UTC by Caroline Cyr La Rose
Modified: 2024-07-04 20:37 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06
Circulation function:


Attachments
Bug 33551: Fix HTML title on patron restrictions page (1.91 KB, patch)
2023-04-18 18:19 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 33551: Fix HTML title on patron restrictions page (1.99 KB, patch)
2023-04-19 08:09 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 33551: Fix HTML title on patron restrictions page (1.98 KB, patch)
2023-04-19 08:11 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 33551: Fix HTML title on patron restrictions page (2.03 KB, patch)
2023-04-20 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33551: (follow-up) Remove spans by using TT filter (1.49 KB, patch)
2023-04-20 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33551: (follow-up) fix display when deleting (1.12 KB, patch)
2023-04-25 20:09 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 33551: Fix HTML title on patron restrictions page (2.10 KB, patch)
2023-04-25 21:39 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 33551: (follow-up) Remove spans by using TT filter (1.56 KB, patch)
2023-04-25 21:39 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
Bug 33551: (follow-up) fix display when deleting (1.19 KB, patch)
2023-04-25 21:39 UTC, Caroline Cyr La Rose
Details | Diff | Splinter Review
[ALTERNATIVE-PATCH] Bug 33551: Remove span from restriction-types.inc (1.39 KB, patch)
2023-05-03 07:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33551: Remove span from restriction-types.inc (1.44 KB, patch)
2023-05-03 20:58 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33551: (QA follow-up) Fix TT filters (1.36 KB, patch)
2023-05-03 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 Caroline Cyr La Rose 2023-04-17 20:52:38 UTC
To test:
1. Enable PatronRestrictionTypes
2. Go to Administration > Patron restrictions
3. Click 'Edit' next to MANUAL
   --> The title of the page (in the browser tab) contains <span> </span>
Comment 1 Lucas Gass (lukeg) 2023-04-18 18:19:43 UTC
Created attachment 149825 [details] [review]
Bug 33551: Fix HTML title on patron restrictions page

To test:
1. Enable PatronRestrictionTypes
2. Go to Administration > Patron restrictions
3. Click 'Edit' next to MANUAL
   --> The title of the page (in the browser tab) contains <span> </span>
4. Apply patch
5. Look at the title of the page when viewing, editing, and deleteing restrictions to make sure they are correct.
Comment 2 Matt Blenkinsop 2023-04-19 08:09:46 UTC
Created attachment 149836 [details] [review]
Bug 33551: Fix HTML title on patron restrictions page

To test:
1. Enable PatronRestrictionTypes
2. Go to Administration > Patron restrictions
3. Click 'Edit' next to MANUAL
   --> The title of the page (in the browser tab) contains <span> </span>
4. Apply patch
5. Look at the title of the page when viewing, editing, and deleteing restrictions to make sure they are correct.

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 3 ByWater Sandboxes 2023-04-19 08:11:39 UTC
Created attachment 149843 [details] [review]
Bug 33551: Fix HTML title on patron restrictions page

To test:
1. Enable PatronRestrictionTypes
2. Go to Administration > Patron restrictions
3. Click 'Edit' next to MANUAL
   --> The title of the page (in the browser tab) contains <span> </span>
4. Apply patch
5. Look at the title of the page when viewing, editing, and deleteing restrictions to make sure they are correct.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 4 Katrin Fischer 2023-04-20 20:58:43 UTC
Created attachment 149994 [details] [review]
Bug 33551: Fix HTML title on patron restrictions page

To test:
1. Enable PatronRestrictionTypes
2. Go to Administration > Patron restrictions
3. Click 'Edit' next to MANUAL
   --> The title of the page (in the browser tab) contains <span> </span>
4. Apply patch
5. Look at the title of the page when viewing, editing, and deleteing restrictions to make sure they are correct.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2023-04-20 20:58:46 UTC
Created attachment 149995 [details] [review]
Bug 33551: (follow-up) Remove spans by using TT filter

This allows up to keep the restriction name in the title
by removing the span tags with a TT filter.

Copied from bug 32061.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2023-04-20 20:59:38 UTC
The first patch removed not only the spans, but also the restriction type (Manual, etc.) from the title. My follow-up adds them back, but still removes the spans. Maybe we could get another sign-off/QA check?
Comment 7 Lucas Gass (lukeg) 2023-04-20 21:05:24 UTC
(In reply to Katrin Fischer from comment #6)
> The first patch removed not only the spans, but also the restriction type
> (Manual, etc.) from the title. My follow-up adds them back, but still
> removes the spans. Maybe we could get another sign-off/QA check?

Probably a good idea.
Comment 8 Katrin Fischer 2023-04-23 09:52:25 UTC
(In reply to Lucas Gass from comment #7)
> (In reply to Katrin Fischer from comment #6)
> > The first patch removed not only the spans, but also the restriction type
> > (Manual, etc.) from the title. My follow-up adds them back, but still
> > removes the spans. Maybe we could get another sign-off/QA check?
> 
> Probably a good idea.

Lucas, do you want to test? We can sign off each others, if you agree to the change of course.
Comment 9 Caroline Cyr La Rose 2023-04-24 13:28:15 UTC
When I try deleting, I have extra quotes.

Confirm deletion of restriction 'Manual''') > ...
Comment 10 Lucas Gass (lukeg) 2023-04-25 20:09:10 UTC
Created attachment 150215 [details] [review]
Bug 33551: (follow-up) fix display when deleting
Comment 11 Lucas Gass (lukeg) 2023-04-25 20:10:07 UTC
(In reply to Caroline Cyr La Rose from comment #9)
> When I try deleting, I have extra quotes.
> 
> Confirm deletion of restriction 'Manual''') > ...

Caroline, I fixed this with a follow-up. Maybe you can try and sign-off since Katrin and I both have patches on this now?

I
Comment 12 Caroline Cyr La Rose 2023-04-25 21:39:29 UTC
Created attachment 150226 [details] [review]
Bug 33551: Fix HTML title on patron restrictions page

To test:
1. Enable PatronRestrictionTypes
2. Go to Administration > Patron restrictions
3. Click 'Edit' next to MANUAL
   --> The title of the page (in the browser tab) contains <span> </span>
4. Apply patch
5. Look at the title of the page when viewing, editing, and deleteing restrictions to make sure they are correct.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 13 Caroline Cyr La Rose 2023-04-25 21:39:31 UTC
Created attachment 150227 [details] [review]
Bug 33551: (follow-up) Remove spans by using TT filter

This allows up to keep the restriction name in the title
by removing the span tags with a TT filter.

Copied from bug 32061.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 14 Caroline Cyr La Rose 2023-04-25 21:39:34 UTC
Created attachment 150228 [details] [review]
Bug 33551: (follow-up) fix display when deleting

Signed-off-by: Caroline Cyr La Rose <caroline.cyr-la-rose@inlibro.com>
Comment 15 Caroline Cyr La Rose 2023-04-25 21:39:52 UTC
Thank you both!
Comment 16 Jonathan Druart 2023-05-02 14:11:52 UTC
the remove span is really not future proof. What if you add an other tag to the include?

You need another PROCESS that would return the translated restriction type from a given code. It will be used from the existing include, and from your <title>
Comment 17 Katrin Fischer 2023-05-02 15:43:17 UTC
(In reply to Jonathan Druart from comment #16)
> the remove span is really not future proof. What if you add an other tag to
> the include?

I agree it's not the prettiest fix, but we already use it in another spot. The span in this case was added to aid translations. Maybe we could add a note to the PROCESS for now? I could also file a separate bug for a 'no span' option. 


> You need another PROCESS that would return the translated restriction type
> from a given code. It will be used from the existing include, and from your
> <title>

Sorry, I read it three times, but still not get it :( Can you explain? Happy to file it as a new bug, but it might not be for me right now.
Comment 18 Jonathan Druart 2023-05-03 07:09:54 UTC
Created attachment 150565 [details] [review]
[ALTERNATIVE-PATCH] Bug 33551: Remove span from restriction-types.inc
Comment 19 Jonathan Druart 2023-05-03 07:10:45 UTC
Why not simply this patch?
Comment 20 Katrin Fischer 2023-05-03 07:30:39 UTC
Ok, you win - that's a good idea.
Comment 21 Katrin Fischer 2023-05-03 20:58:48 UTC
Created attachment 150617 [details] [review]
Bug 33551: Remove span from restriction-types.inc

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Katrin Fischer 2023-05-03 20:58:51 UTC
Created attachment 150618 [details] [review]
Bug 33551: (QA follow-up) Fix TT filters

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 23 Katrin Fischer 2023-05-03 21:00:07 UTC
I've tested in the GUI, but also updated the po files to verify that the changes show up as strings and can be translated. Also fixed some TT filters ;)
Comment 24 Tomás Cohen Arazi (tcohen) 2023-05-05 13:15:40 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 25 Martin Renvoize (ashimema) 2023-05-12 14:42:52 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 26 Lucas Gass (lukeg) 2023-06-13 15:54:59 UTC
Missing dependencies for 22.05.x, no backport.