Summary: | Label maker font list is not configurable | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Label/patron card printing | Assignee: | David Cook <dcook> |
Status: | Failed QA --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | bgkriegel, david, egpetridis, fridolin.somers, severine.queune |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
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: | |
Circulation function: | |||
Attachments: |
Bug 25384: Use koha-conf.xml to configure label maker font list
Bug 25384: Use koha-conf.xml to configure label maker font list Bug 25384: Use koha-conf.xml to configure label maker font list Bug 25384: Use dclone when copying font lookup hash Bug 25384: Use koha-conf.xml to configure label maker font list Bug 25384: Use dclone when copying font lookup hash |
Description
David Cook
2020-05-06 01:35:09 UTC
Created attachment 104390 [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 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 :) (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. The concept looks sound. I have no way of testing at the moment. (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. (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. (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! Created attachment 104505 [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: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Works as described, no errors (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. (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. (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 :) Patches no longer apply cleanly, please rebase! 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 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. 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", ... ? (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. (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 :/. (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 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 (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 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 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> 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> 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. (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? (In reply to David Cook from comment #25) > Does that make sense? Thanks David, it does! Have a good break over January. |