Bug 25384 - Label maker font list is not configurable
Summary: Label maker font list is not configurable
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-06 01:35 UTC by David Cook
Modified: 2024-01-02 03:08 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement lets you configure the label maker font list and add new fonts. It fetches the <ttf> section from koha-conf.xml, and looks for entries with a "name" attribute: - For an existing default font, it will rename that font in the font list. - For a new font, it will add it to the font list in the label creator pages. - If you don't change anything in koha-conf.xml, then there is no behaviour change. Notes: 1. To configure the name displayed in the label maker font list, add a name element in koha-conf.xml to the font in the <ttf> section (for example: name="Times New Roman"). 2. To add a new font, add an entry in the <ttf> section (for example: <font type="A" name="Arial">/usr/share/fonts/truetype/liberation/LiberationSans-Regular.ttf</font>).
Version(s) released in:


Attachments
Bug 25384: Use koha-conf.xml to configure label maker font list (5.67 KB, patch)
2020-05-06 03:31 UTC, David Cook
Details | Diff | Splinter Review
Bug 25384: Use koha-conf.xml to configure label maker font list (5.76 KB, patch)
2020-05-07 12:20 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 25384: Use koha-conf.xml to configure label maker font list (5.68 KB, patch)
2020-08-17 00:34 UTC, David Cook
Details | Diff | Splinter Review
Bug 25384: Use dclone when copying font lookup hash (2.25 KB, patch)
2020-08-17 00:34 UTC, David Cook
Details | Diff | Splinter Review
Bug 25384: Use koha-conf.xml to configure label maker font list (5.72 KB, patch)
2024-01-01 23:38 UTC, David Nind
Details | Diff | Splinter Review
Bug 25384: Use dclone when copying font lookup hash (2.30 KB, patch)
2024-01-01 23:38 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2020-05-06 01:35:09 UTC
The label maker font list is hard-coded to 12 arbitrary choices. It would be better if we could configure the font choices in the user interface (since we can already configure the truetype font files in koha-conf.xml).
Comment 1 David Cook 2020-05-06 03:31:12 UTC Comment hidden (obsolete)
Comment 2 Bernardo Gonzalez Kriegel 2020-05-06 12:20:27 UTC
Hi David,
it's been a long time since I visited this code :)

Patch works, you can also use a longer code in font entry, eg.
<font type="Arial" name="Arial">/usr/...</font>

also an old bug resurfaces, if you select oblique title in the layout,
ERROR in koha-conf.xml -- missing <font type="ArialO">/path/to/font.ttf</font>

See Bug 8375#c83

But it works :)
Comment 3 Chris Nighswonger 2020-05-06 12:26:05 UTC
(In reply to David Cook from comment #0)
> The label maker font list is hard-coded to 12 arbitrary choices.

FTR: These are not arbitrary. Rather they are 12 of the 14 basic fonts spec'd in the Adobe PDF standard. 13 of them are commonly found on postscript printers. Hence their hard-coding and inclusion as default fonts.

I'm creating this comment because some of this is arcane knowledge but important to inform future changes.
Comment 4 Chris Nighswonger 2020-05-06 12:28:29 UTC
The concept looks sound. I have no way of testing at the moment.
Comment 5 David Cook 2020-05-07 00:52:08 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #2)
> Hi David,
> it's been a long time since I visited this code :)
> 
> Patch works, you can also use a longer code in font entry, eg.
> <font type="Arial" name="Arial">/usr/...</font>
> 
> also an old bug resurfaces, if you select oblique title in the layout,
> ERROR in koha-conf.xml -- missing <font
> type="ArialO">/path/to/font.ttf</font>
> 
> See Bug 8375#c83
> 
> But it works :)

Yikes... can you tell me more about re-creating that one? I don't really understand from the comments on that bug.
Comment 6 David Cook 2020-05-07 00:56:26 UTC
(In reply to Chris Nighswonger from comment #3)
> (In reply to David Cook from comment #0)
> > The label maker font list is hard-coded to 12 arbitrary choices.
> 
> FTR: These are not arbitrary. Rather they are 12 of the 14 basic fonts
> spec'd in the Adobe PDF standard. 13 of them are commonly found on
> postscript printers. Hence their hard-coding and inclusion as default fonts.
> 
> I'm creating this comment because some of this is arcane knowledge but
> important to inform future changes.

Thanks for the clarification, Chris! My apologies for calling them arbitrary. It might be worthwhile encoding that information in comments in the code even, as I could see this knowledge getting lost here.
Comment 7 David Cook 2020-05-07 00:57:23 UTC
(In reply to Chris Nighswonger from comment #4)
> The concept looks sound. I have no way of testing at the moment.

No worries. Thanks for looking at it in any case. I'm glad to have your review at the very least. Much appreciated!
Comment 8 Bernardo Gonzalez Kriegel 2020-05-07 12:20:47 UTC Comment hidden (obsolete)
Comment 9 Bernardo Gonzalez Kriegel 2020-05-07 12:30:43 UTC
(In reply to David Cook from comment #5)
> Yikes... can you tell me more about re-creating that one? I don't really
> understand from the comments on that bug.

Suppose you select Arial (code A) in a layout, and you set the checkbox for 'Oblique title', then C4/Labels/Label.pm will try to find an 'oblique' variant for your selected font by adding an 'O' or 'I' to the code, 
eg. 
Times Roman (code T) -> Times Roman Italic (code TI)
Arial (code A) -> Arial oblique (code AO)
Last code is undefined and you will find a warn in intranet logs, that's all, but the export will work

Signed.
Comment 10 Chris Nighswonger 2020-05-07 12:33:03 UTC
(In reply to Bernardo Gonzalez Kriegel from comment #9)
> (In reply to David Cook from comment #5)
> > Yikes... can you tell me more about re-creating that one? I don't really
> > understand from the comments on that bug.
> 
> Suppose you select Arial (code A) in a layout, and you set the checkbox for
> 'Oblique title', then C4/Labels/Label.pm will try to find an 'oblique'
> variant for your selected font by adding an 'O' or 'I' to the code, 

An additional improvement would be to have the UI hide the oblique (italic) option if none is available for the font selected.
Comment 11 Bernardo Gonzalez Kriegel 2020-05-07 12:47:39 UTC
(In reply to Chris Nighswonger from comment #10)
> An additional improvement would be to have the UI hide the oblique (italic)
> option if none is available for the font selected.

Good idea :)
Comment 12 Katrin Fischer 2020-08-16 09:33:59 UTC
Patches no longer apply cleanly, please rebase!
Comment 13 David Cook 2020-08-17 00:34:48 UTC
Created attachment 108377 [details] [review]
Bug 25384: Use koha-conf.xml to configure label maker font list

This patch fetches the "ttf" element in koha-conf.xml, and looks
for entries with a "name" attribute. If it's an existing default
font, it will rename that font. If it's a new font, it will add
it to the font list in the label creator pages.

If you don't change anything in koha-conf.xml, then there is no
behaviour change and the existing unit tests will all complete
as normal.

Test plan:
1) Apply patch
2) Edit koha-conf.xml and add a "name" attribute of "Times New Awesome"
next to the font "type" "TR".
3) Edit koha-conf.xml and add the following entry to the "ttf" element:
<font type="A" name="Arial">/usr/share/fonts/truetype/liberation/LiberationSans-Regular.ttf</font>
4) Clear memcached (if using memcached)
    a. e.g. echo 'flush_all' | nc -q 1 memcached 11211
5) Restart Plack (if using Plack)
    a. e.g. koha-plack --restart kohadev
6) Create or edit a Label maker layout
    a. e.g. /cgi-bin/koha/labels/label-edit-layout.pl?op=edit&element_id=17
7) Note that "Times-Roman" now says "Times New Awesome" instead
8) Note that there is now an "Arial" option at the bottom of the list
9) Try to export a label batch using a layout using "Arial"
10) Note that the text is output with a font style similar to Arial
Comment 14 David Cook 2020-08-17 00:34:52 UTC
Created attachment 108378 [details] [review]
Bug 25384: Use dclone when copying font lookup hash

This patch includes the logic from Bug 25852. That is,
use dclone to copy the hash used as the font lookup.

While the original patch perfectly copied the top level
keys, the next level down were hash references (due to Perl's
inherent hash design), and changes to the 2nd level of depth
would be changes to these references which would impact
the original hash.
Comment 15 Fridolin Somers 2020-09-03 09:42:45 UTC
Nice job ;)

Since default font in koha-conf.xml is "DejaVu", shouldn't we define it as default for font names, instead of MS-non-opensource fonts "Times-Roman", ... ?
Comment 16 Chris Nighswonger 2020-09-03 12:02:51 UTC
(In reply to Fridolin SOMERS from comment #15)
> Nice job ;)
> 
> Since default font in koha-conf.xml is "DejaVu", shouldn't we define it as
> default for font names, instead of MS-non-opensource fonts "Times-Roman",
> ... ?

DejaVu does not appear to be one of the 14 base fonts. Be careful about making this change before understanding how PDFs and these fonts relate. Here are a couple of resources:

https://kbpdfstudio.qoppa.com/standard-14-pdf-fonts/

https://www.adobe.com/devnet/font.html

Switching to a non-base font could cause problems on some systems. The pdf code in Koha is set to embed fonts which should avoid most potential issues. But we should be aware that defaulting to a non-base font "might" introduce an issue depending on the system used to open the resulting PDF.

FTR: This is not an objection to Fridolin's suggestion. I'm all for cutting MS out of the picture altogether whenever possible.
Comment 17 David Cook 2020-09-04 03:49:48 UTC
(In reply to Fridolin SOMERS from comment #15)
> Nice job ;)
> 
> Since default font in koha-conf.xml is "DejaVu", shouldn't we define it as
> default for font names, instead of MS-non-opensource fonts "Times-Roman",
> ... ?

I think that would be scope creep here, but I'm not opposed it overall. Except that end-users probably wouldn't understand what "DejaVu" means :/.
Comment 18 Fridolin Somers 2020-09-10 07:07:35 UTC
(In reply to David Cook from comment #17)
> (In reply to Fridolin SOMERS from comment #15)
> > Nice job ;)
> > 
> > Since default font in koha-conf.xml is "DejaVu", shouldn't we define it as
> > default for font names, instead of MS-non-opensource fonts "Times-Roman",
> > ... ?
> 
> I think that would be scope creep here, but I'm not opposed it overall.
> Except that end-users probably wouldn't understand what "DejaVu" means :/.

We may add a like to wiki https://en.wikipedia.org/wiki/DejaVu_fonts
Comment 19 Manos PETRIDIS 2022-03-06 11:17:15 UTC
Could https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28806 be related to the fonts used? I can't find any reference to fonts in my library's koha-conf.xml

MP
Comment 20 Katrin Fischer 2022-03-06 11:31:32 UTC
(In reply to Manos PETRIDIS from comment #19)
> Could https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28806 be
> related to the fonts used? I can't find any reference to fonts in my
> library's koha-conf.xml
> 
> MP

It should look something like this with the FONT_DIR replaced:
https://git.koha-community.org/Koha-community/Koha/src/branch/master/etc/koha-conf.xml#L132
Comment 21 Manos PETRIDIS 2022-03-06 15:00:19 UTC
Thenk you very much for the pointer!

MP(In reply to Katrin Fischer from comment #20)
> (In reply to Manos PETRIDIS from comment #19)
> > Could https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28806 be
> > related to the fonts used? I can't find any reference to fonts in my
> > library's koha-conf.xml
> > 
> > MP
> 
> It should look something like this with the FONT_DIR replaced:
> https://git.koha-community.org/Koha-community/Koha/src/branch/master/etc/
> koha-conf.xml#L132

Thank you very much Katrin for the pointer!

MP
Comment 22 David Nind 2024-01-01 23:38:15 UTC
Created attachment 160390 [details] [review]
Bug 25384: Use koha-conf.xml to configure label maker font list

This patch fetches the "ttf" element in koha-conf.xml, and looks
for entries with a "name" attribute. If it's an existing default
font, it will rename that font. If it's a new font, it will add
it to the font list in the label creator pages.

If you don't change anything in koha-conf.xml, then there is no
behaviour change and the existing unit tests will all complete
as normal.

Test plan:
1) Apply patch
2) Edit koha-conf.xml and add a "name" attribute of "Times New Awesome"
next to the font "type" "TR".
3) Edit koha-conf.xml and add the following entry to the "ttf" element:
<font type="A" name="Arial">/usr/share/fonts/truetype/liberation/LiberationSans-Regular.ttf</font>
4) Clear memcached (if using memcached)
    a. e.g. echo 'flush_all' | nc -q 1 memcached 11211
5) Restart Plack (if using Plack)
    a. e.g. koha-plack --restart kohadev
6) Create or edit a Label maker layout
    a. e.g. /cgi-bin/koha/labels/label-edit-layout.pl?op=edit&element_id=17
7) Note that "Times-Roman" now says "Times New Awesome" instead
8) Note that there is now an "Arial" option at the bottom of the list
9) Try to export a label batch using a layout using "Arial"
10) Note that the text is output with a font style similar to Arial

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 David Nind 2024-01-01 23:38:18 UTC
Created attachment 160391 [details] [review]
Bug 25384: Use dclone when copying font lookup hash

This patch includes the logic from Bug 25852. That is,
use dclone to copy the hash used as the font lookup.

While the original patch perfectly copied the top level
keys, the next level down were hash references (due to Perl's
inherent hash design), and changes to the 2nd level of depth
would be changes to these references which would impact
the original hash.

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 David Nind 2024-01-01 23:54:47 UTC
Testing notes (using KTD):

1. The patches still apply.

2. Everything works as per the test plan.

I was not sure about some of the later comments, so please feel free to change the status if any of these need addressing in this bug.

I changed the assignee to David Cook, feel free to change it if that is no longer the case.

I've had a go at the release notes - not sure that I've have got this right. Feel free to edit it!

Just to confirm that I understand this change correctly. Before this patch, you couldn't configure how the font names were displayed (they were hard coded), but you could still add new fonts? This change now lets you configure how the fonts are displayed in the lists, and you can still add new fonts.
Comment 25 David Cook 2024-01-02 01:11:00 UTC
(In reply to David Nind from comment #24)
> Testing notes (using KTD):
> 
> 1. The patches still apply.
> 
> 2. Everything works as per the test plan.
> 
> I was not sure about some of the later comments, so please feel free to
> change the status if any of these need addressing in this bug.
> 
> I changed the assignee to David Cook, feel free to change it if that is no
> longer the case.
> 
> I've had a go at the release notes - not sure that I've have got this right.
> Feel free to edit it!
> 
Thanks, David. You are a champion!

I think that this change should have unit tests, so I'm going to Failed QA my own patches, unfortunately. But your work here makes me want to return and add them!

I'm not in the office much in January, but I'll add it to my list!

> Just to confirm that I understand this change correctly. Before this patch,
> you couldn't configure how the font names were displayed (they were hard
> coded), but you could still add new fonts? This change now lets you
> configure how the fonts are displayed in the lists, and you can still add
> new fonts.

Before this patch, you couldn't configure how the font names and you couldn't add new fonts all together. You could only change the font file associated with a certain font name. 

So if you wanted to use a font type that wasn't in the list, you'd have to just override an existing font, and perhaps use some Javascript to change the name in the list to the appropriate name.

Does that make sense?
Comment 26 David Nind 2024-01-02 03:08:34 UTC
(In reply to David Cook from comment #25)
 
> Does that make sense?

Thanks David, it does!

Have a good break over January.